Bug 75657 – WTF::yield() conflicts with unistd.h when building with Solaris Studio 12.3

• Status: RESOLVED
• Resolution: WONTFIX
• Component: Web Template Framework

[Pavel Heimlich (hajma)]

when building qt 4.8, the included webkit fails to build with the below error.

I found this patch helped me to move on, but I don't really speak c++ so I'm not sure if this is the correct way:
--- ./wtf/Threading.h.orig      Thu Jan  5 23:10:38 2012
+++ ./wtf/Threading.h   Thu Jan  5 23:11:14 2012
@@ -113,6 +113,6 @@
 using WTF::currentThread;
 using WTF::detachThread;
 using WTF::waitForThreadCompletion;
-using WTF::yield;
+//using WTF::yield;
 
 #endif // Threading_h



I'm ready to come up with proper patch along the webkit rules, but please let me know if the direction is ok.

-bash-4.1$ cd ~/packages/BUILD/qt-4.8.0/i386/qt-everywhere-opensource-src-4.8.0/src/3rdparty/webkit/Source/JavaScriptCore/
-bash-4.1$ gmake VERBOSE=1
/opt/test2/packages/BUILD/qt-4.8.0/i386/qt-everywhere-opensource-src-4.8.0/bin/qmake -spec ../../../../../mkspecs/solaris-cc -o Makefile JavaScriptCore.pro
/opt/test2/SolarisStudio12.3-solaris-x86-bin/solarisstudio12.3/bin/CC -c -I../../../../../mkspecs/solaris-cc -I. -I../../../../../include/QtCore -I../../../../../include -I. -I../../Source -I../ThirdParty -Iassembler -Ibytecode -Ibytecompiler -Iheap -Idfg -Idebugger -Iinterpreter -Ijit -Iparser -Iprofiler -Iruntime -Iwtf -Iwtf/gobject -I/opt/test2/packages/BUILD/qt-4.8.0/i386/qt-everywhere-opensource-src-4.8.0/src/3rdparty/webkit/Source/JavaScriptCore/wtf/symbian -Iwtf/unicode -Iyarr -IAPI -IForwardingHeaders -Igenerated -I../../Source -I../../include -I/include/QtWebKit -I/include -I.moc/debug-static -I/usr/sfw/include -D_REENTRANT -DNDEBUG -D__EXTENSIONS__ -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_POSIX_PTHREAD_SEMANTICS -DSOLARIS -DNO_DEBUG -D_UNICODE -DUNICODE -D_RWSTD_REENTRANT -D_XOPEN_SOURCE=500 -D_XPG5 -I/opt/kde4/include -m32 -xarch=sse2 -features=extensions,nestedaccess,tmplrefstatic -template=geninlinefuncs -s -xlang=c99 -xustr=ascii_utf16_ushort -Qoption ccfe -features=gcc -Qoption ccfe -features=zla -Qoption ccfe ++boolflag:sunwcch=false -R/opt/kde4/lib -library=stdcxx4 -lCrun -I/usr/include/libpng14 -I/opt/kde4/include -I/usr/mysql/5.1/include/mysql -I/usr/include/libmng -I/usr/include/dbus-1.0 -I/usr/lib/dbus-1.0/include -I/usr/include/odbc -KPIC -mt -DNDEBUG -DBUILDING_QT__=1 -DQT_ASCII_CAST_WARNINGS -DBUILDING_JavaScriptCore -DBUILDING_WTF -D_LARGEFILE64_SOURCE -D_LARGEFILE_SOURCE -DQT_CORE_LIB -DQT_HAVE_MMX -DQT_HAVE_SSE -DQT_HAVE_MMXEXT -DQT_HAVE_SSE2 -DQT_HAVE_SSE3 -DQT_SHARED -o .obj/debug-static/YarrInterpreter.o yarr/YarrInterpreter.cpp
"./wtf/text/StringImplBase.h", line 46: Warning: Identifier expected instead of "}".
"yarr/YarrPattern.h", line 86: Warning: Identifier expected instead of "}".
"yarr/YarrPattern.h", line 99: Warning: Identifier expected instead of "}".
"yarr/YarrPattern.h", line 113: Warning: Types cannot be declared in anonymous union.
"yarr/YarrPattern.h", line 201: Warning: type hides JSC::Yarr::PatternTerm::type.
"yarr/YarrInterpreter.h", line 74: Warning: Identifier expected instead of "}".
"yarr/YarrInterpreter.h", line 85: Warning: Types cannot be declared in anonymous union.
"yarr/YarrInterpreter.h", line 99: Warning: Types cannot be declared in anonymous union.
"./wtf/OSAllocator.h", line 43: Warning: Identifier expected instead of "}".
"/usr/include/unistd.h", line 533: Error: WTF::yield() was declared before with a different language.
"yarr/YarrInterpreter.cpp", line 153: Warning: output hides JSC::Yarr::Interpreter::output.
1 Error(s) and 10 Warning(s) detected.

[Alexey Proskuryakov]

Such change would break the build, as is thus not acceptable.

However, I'm not sure if we really need to implement yield() - it looks like it's only used once, as added in bug 42843, without much explanation of why it was needed.

[David Levin]

Michael, do you know why the yield statement is needed in the sql code from bug 42843?

Could it be removed? (Or replace with something else?)

[Michael Nordman]

(In reply to comment #2)
> Michael, do you know why the yield statement is needed in the sql code from bug 42843?
> 
> Could it be removed? (Or replace with something else?)

Looks like the problem is that "using WTF::yeild" in a .h file puts a very generic function name in the global namespace. We could change the name of the WTF::yeild() function to be less generic. Or better yet, avoid 'using' it and require callers (ap says there's only one) to fully qualify the function name.

[David Levin]

(In reply to comment #3)
> (In reply to comment #2)
> > Michael, do you know why the yield statement is needed in the sql code from bug 42843?
> > 
> > Could it be removed? (Or replace with something else?)
> 
> Looks like the problem is that "using WTF::yeild" in a .h file puts a very generic function name in the global namespace. We could change the name of the WTF::yeild() function to be less generic. Or better yet, avoid 'using' it and require callers (ap says there's only one) to fully qualify the function name.

Michael, here is the one instance:   http://trac.webkit.org/browser/trunk/Source/WebCore/platform/sql/SQLiteDatabase.cpp#L128

which is why I was asking you :).

Maybe the answer is: Feel free to submit a patch which removes the "using" statement and changes SQLiteDatabase.cpp to WTF::yield?

[Alexey Proskuryakov]

> Maybe the answer is: Feel free to submit a patch which removes the "using" statement and changes SQLiteDatabase.cpp to WTF::yield?

I don't think that this is the best solution. We export all public functions from WTF into global namespace, so having an exception because of a rare platform seems wrong.

Renaming the function or removing it altogether seems better.

[Michael Nordman]

(In reply to comment #5)
> > Maybe the answer is: Feel free to submit a patch which removes the "using" statement and changes SQLiteDatabase.cpp to WTF::yield?
> 
> I don't think that this is the best solution. We export all public functions from WTF into global namespace, so having an exception because of a rare platform seems wrong.

Interesting that this comes up with the "[chromium-dev] Pros and cons of "using namespace" (WAS: Consensus on when to use "using" directives)" discussion active.

> Renaming the function or removing it altogether seems better.

Feel free to submit a patch which removes the "using" statement and explicitly calls WTF::yeild, or to rename it to something that doesn't collide like wtfYield(). Pretty clear that yield() is used here to prevent this method from starving other threads of cpu. It's probably not a good idea to just remove it.

120    void SQLiteDatabase::interrupt()
121    {
122        m_interrupted = true;
123        while (!m_lockingMutex.tryLock()) {
124            MutexLocker locker(m_databaseClosingMutex);
125            if (!m_db)
126                return;
127            sqlite3_interrupt(m_db);
128            yield();
129        }
130    
131        m_lockingMutex.unlock();
132    }

[Ahmad Saleem]

Qt framework support for Webkit build is no longer present and there is no active build bots for QtWebkit. Can this be marked as "RESOLVED WONTFIX"?

Further - in Webkit Github source, there is only following reference to "yield' in "threading.h" file:

https://github.com/WebKit/WebKit/blob/fff69b8fe7b721effabdc81a70cbdf9926de3262/Source/WTF/wtf/Threading.h#L200

Additionally, based on Comment 04, the file does not have any "USING" word anymore (used CMD+F to find):

https://github.com/WebKit/WebKit/blob/6fe0158b88e945a7d11ae3d3c68ee8acd6cbef81/Source/WebCore/platform/sql/SQLiteDatabase.cpp

I think this can be closed. Thanks!