Bug 95468 – [Qt] QNX triggers compile errors in ANGLE due to missing std::

• Component: WebKit Qt
• Status: RESOLVED WONTFIX

[Milian Wolff]

Various constructs in the ANGLE third party library reference code from cstring or similar headers without the explicit std:: namespace identifier. On other platforms this is apparently not an issue, one guess from Simon is:

[13:30] <tronical> milian: I suppose an implicit inclusion of string.h from somewhere, so that the extern "C" variant of strlen is chosen?

On QNX though, this is not the case and various compile errors arise. Expect a patch from me.

[Milian Wolff]

Created attachment 161657 [details]
Patch

[WebKit Review Bot]

Attachment 161657 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/ThirdParty/ANGLE/ChangeLog', u'Sour..." exit_code: 1
Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/DirectiveParser.cpp:712:  More than one command on the same line in if  [whitespace/parens] [4]
Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/DirectiveParser.cpp:762:  More than one command on the same line in if  [whitespace/parens] [4]
Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/DirectiveParser.cpp:771:  More than one command on the same line in if  [whitespace/parens] [4]
Total errors found: 3 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Milian Wolff]

The style errors existed in ANGLE codebase already, I didn't change them on purpose. Should I though?

[Pravin D]

(In reply to comment #3)
> The style errors existed in ANGLE codebase already, I didn't change them on purpose. Should I though?
> 

The previous code also had this problem(in correct style) i guess. 
The webkit style check script applies only to the code that has been changed. That is only to code added/modified by your patch. 

just bring the line content after the if() to the line below.
See 
http://www.webkit.org/coding/coding-style.html

Hope this helps...

[Milian Wolff]

This means though that the patch will break the code style of the surrounding file, just to get it accepted in WebKit? Assuming I upstream the patch as-is to ANGLE, would I still need to change it to get it into WebKit and thus create a diversion?

[Darin Adler]

Comment on attachment 161657 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=161657&action=review

> Source/ThirdParty/ANGLE/ChangeLog:12
> +        Add the explicit std:: qualifier to uses of std functions and types.
> +        On other platforms this is apparently not required due to implicit
> +        inclusion of "string.h"-style headers containing extern "C" variants.
> +        On QNX at least this is not the case though, resulting in compile
> +        errors.

Wouldn't a simpler way to fix this would be to add one or more includes of <string.h>?

[Milian Wolff]

also blocks 74040

[Milian Wolff]

Upstream patch: https://codereview.appspot.com/6653048

[Simon Hausmann]

(In reply to comment #6)
> (From update of attachment 161657 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=161657&action=review
> 
> > Source/ThirdParty/ANGLE/ChangeLog:12
> > +        Add the explicit std:: qualifier to uses of std functions and types.
> > +        On other platforms this is apparently not required due to implicit
> > +        inclusion of "string.h"-style headers containing extern "C" variants.
> > +        On QNX at least this is not the case though, resulting in compile
> > +        errors.
> 
> Wouldn't a simpler way to fix this would be to add one or more includes of <string.h>?

Millian, did you see Darin's comment? If that works then it would seem like a much simpler solution :)

[Milian Wolff]

Yes, I saw Darin's comment, but wonder why it should be done instead of adding the explicit std:: qualified identifier.

Example: When trying to compile DirectiveParser.cpp into DirectiveParser.o, we get (without this patch):

qcc -Vgcc_ntoarmv7le -c -Wno-psabi -lang-c++ -O2 -fvisibility=hidden -fvisibility-inlines-hidden -fPIC -D_REENTRANT -DQT_NO_CLIPBOARD -DQT_NO_LIBUDEV -DQT_NO_EVDEV -DQT_NO_XCB -DBUILDING_QT__=1 -DNDEBUG -DENABLE_3D_RENDERING=1 -DENABLE_BLOB=1 -DENABLE_CHANNEL_MESSAGING=1 -DENABLE_CSS_BOX_DECORATION_BREAK=1 -DENABLE_CSS_COMPOSITING=1 -DENABLE_CSS_EXCLUSIONS=1 -DENABLE_CSS_FILTERS=1 -DENABLE_CSS_IMAGE_SET=1 -DENABLE_CSS_REGIONS=1 -DENABLE_CSS_STICKY_POSITION=1 -DENABLE_DATALIST_ELEMENT=1 -DENABLE_DETAILS_ELEMENT=1 -DENABLE_FAST_MOBILE_SCROLLING=1 -DENABLE_FILTERS=1 -DENABLE_FTPDIR=1 -DENABLE_FULLSCREEN_API=1 -DENABLE_GESTURE_EVENTS=1 -DENABLE_ICONDATABASE=1 -DENABLE_IFRAME_SEAMLESS=1 -DENABLE_INPUT_TYPE_COLOR=1 -DENABLE_INSPECTOR=1 -DENABLE_INSPECTOR_SERVER=1 -DENABLE_JAVASCRIPT_DEBUGGER=1 -DENABLE_LEGACY_NOTIFICATIONS=1 -DENABLE_LEGACY_VIEWPORT_ADAPTION=1 -DENABLE_LEGACY_VENDOR_PREFIXES=1 -DENABLE_METER_ELEMENT=1 -DENABLE_NOTIFICATIONS=1 -DENABLE_PAGE_VISIBILITY_API=1 -DENABLE_PROGRESS_ELEMENT=1 -DENABLE_RESOLUTION_MEDIA_QUERY=1 -DENABLE_REQUEST_ANIMATION_FRAME=1 -DENABLE_SHARED_WORKERS=1 -DENABLE_SMOOTH_SCROLLING=1 -DENABLE_SQL_DATABASE=1 -DENABLE_SVG=1 -DENABLE_SVG_FONTS=1 -DENABLE_TOUCH_ADJUSTMENT=1 -DENABLE_TOUCH_EVENTS=1 -DENABLE_WEB_SOCKETS=1 -DENABLE_WEB_TIMING=1 -DENABLE_WORKERS=1 -DWTF_USE_TILED_BACKING_STORE=1 -DHAVE_QTQUICK=1 -DHAVE_QTPRINTSUPPORT=1 -DHAVE_QSTYLE=1 -DHAVE_QTTESTLIB=1 -DWTF_USE_ZLIB=1 -DPLUGIN_ARCHITECTURE_UNSUPPORTED=1 -DWTF_USE_3D_GRAPHICS=1 -DENABLE_WEBGL=1 -DENABLE_CSS_SHADERS=1 -DENABLE_TOUCH_SLIDER=1 -DENABLE_ACCELERATED_2D_CANVAS=0 -DENABLE_ANIMATION_API=0 -DENABLE_BATTERY_STATUS=0 -DENABLE_CSP_NEXT=0 -DENABLE_CSS_GRID_LAYOUT=0 -DENABLE_CSS_HIERARCHIES=0 -DENABLE_CSS_IMAGE_ORIENTATION=0 -DENABLE_CSS_IMAGE_RESOLUTION=0 -DENABLE_CSS_VARIABLES=0 -DENABLE_CSS3_CONDITIONAL_RULES=0 -DENABLE_CSS3_TEXT=0 -DENABLE_DASHBOARD_SUPPORT=0 -DENABLE_DATAGRID=0 -DENABLE_DATA_TRANSFER_ITEMS=0 -DENABLE_DEVICE_ORIENTATION=0 -DENABLE_DIRECTORY_UPLOAD=0 -DENABLE_DOWNLOAD_ATTRIBUTE=0 -DENABLE_FILE_SYSTEM=0 -DENABLE_GAMEPAD=0 -DENABLE_GEOLOCATION=0 -DENABLE_HIGH_DPI_CANVAS=0 -DENABLE_INDEXED_DATABASE=0 -DENABLE_INPUT_SPEECH=0 -DENABLE_INPUT_TYPE_DATE=0 -DENABLE_INPUT_TYPE_DATETIME=0 -DENABLE_INPUT_TYPE_DATETIMELOCAL=0 -DENABLE_INPUT_TYPE_MONTH=0 -DENABLE_INPUT_TYPE_TIME=0 -DENABLE_INPUT_TYPE_WEEK=0 -DENABLE_LEGACY_CSS_VENDOR_PREFIXES=0 -DENABLE_LINK_PREFETCH=0 -DENABLE_LINK_PRERENDER=0 -DENABLE_MATHML=0 -DENABLE_MEDIA_SOURCE=0 -DENABLE_MEDIA_STATISTICS=0 -DENABLE_MEDIA_STREAM=0 -DENABLE_MHTML=0 -DENABLE_MICRODATA=0 -DENABLE_MUTATION_OBSERVERS=0 -DENABLE_NAVIGATOR_CONTENT_UTILS=0 -DENABLE_NETSCAPE_PLUGIN_API=0 -DENABLE_NETWORK_INFO=0 -DENABLE_ORIENTATION_EVENTS=0 -DENABLE_QUOTA=0 -DENABLE_SCRIPTED_SPEECH=0 -DENABLE_SHADOW_DOM=0 -DENABLE_STYLE_SCOPED=0 -DENABLE_SVG_DOM_OBJC_BINDINGS=0 -DENABLE_TEXT_AUTOSIZING=0 -DENABLE_TEXT_NOTIFICATIONS_ONLY=0 -DENABLE_TOUCH_ICON_LOADING=0 -DENABLE_UNDO_MANAGER=0 -DENABLE_VIBRATION=0 -DENABLE_VIDEO=0 -DENABLE_VIDEO_TRACK=0 -DENABLE_WEB_AUDIO=0 -DENABLE_XSLT=0 -DBUILDING_ANGLE -DBUILDING_WEBKIT -DQT_ASCII_CAST_WARNINGS -DQT_NO_EXCEPTIONS -D_LARGEFILE64_SOURCE -D_LARGEFILE_SOURCE -DQT_NO_DEBUG -I/home/milian/projects/qt5/install-qnx650/mkspecs/qnx-armv7le-qcc -I/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE -I.moc/debug-shared -I/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/include/GLSLANG -I/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src -I/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new -I/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/include -I/home/milian/projects/qt5/qtwebkit/Source -I/home/milian/projects/qt5/build-qnx650/qtwebkit/Source/include -I/home/milian/projects/qt5/install-qnx650/include -I/home/milian/projects/qt5/install-qnx650/include/QtScript -I/opt/qnx650/target/qnx6/usr/include -I/opt/qnx650/target/qnx6/usr/include/freetype2 -I. -o obj/debug/src/compiler/preprocessor/new/DirectiveParser.o /home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/DirectiveParser.cpp
In file included from /home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/DirectiveParser.cpp:16:
/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/MacroExpander.h:55: error: 'size_t' does not name a type                                                                                                                                         
/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/MacroExpander.h: In constructor 'pp::MacroExpander::MacroContext::MacroContext()':
/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/MacroExpander.h:58: error: class 'pp::MacroExpander::MacroContext' does not have any field named 'index'                                                                                         
/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/MacroExpander.h: In member function 'bool pp::MacroExpander::MacroContext::empty() const':
/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/MacroExpander.h:59: error: ISO C++ forbids comparison between pointer and integer                                                                                                                
/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/MacroExpander.h: In member function 'const pp::Token& pp::MacroExpander::MacroContext::get()':
/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/MacroExpander.h:60: error: ISO C++ forbids incrementing a pointer of type 'const char* (*)(const char*, int)'                                                                                    
/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/MacroExpander.h:60: error: lvalue required as increment operand                                                                                                                                  
/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/MacroExpander.h: In member function 'void pp::MacroExpander::MacroContext::unget()':
/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/MacroExpander.h:61: error: ISO C++ forbids decrementing a pointer of type 'const char* (*)(const char*, int)'                                                                                    
/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/MacroExpander.h:61: error: lvalue required as decrement operand                                                                                                                                  
/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/DirectiveParser.cpp: In member function 'void pp::DirectiveParser::parseVersion(pp::Token*)':
/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/DirectiveParser.cpp:712: error: 'atoi' was not declared in this scope                                                                                                                            
/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/DirectiveParser.cpp: In member function 'void pp::DirectiveParser::parseLine(pp::Token*)':
/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/DirectiveParser.cpp:762: error: 'atoi' was not declared in this scope                                                                                                                            
/home/milian/projects/qt5/qtwebkit/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/DirectiveParser.cpp:771: error: 'atoi' was not declared in this scope                                                                                                                            
cc: /opt/qnx650/host/linux/x86/usr/lib/gcc/arm-unknown-nto-qnx6.5.0eabi/4.4.2/cc1plus error 1
make[2]: *** [obj/debug/src/compiler/preprocessor/new/DirectiveParser.o] Error 1
make[2]: Leaving directory `/home/milian/projects/qt5/build-qnx650/qtwebkit/Source/ThirdParty/ANGLE'

Where would you add the include to string.h? Into MacroExpander.h?  DirectiveParser.cpp? But then again, if I'm patching ANGLE sources, I can also fix it by adding the std:: namespace identifier. Anyhow, I'll ask the ANGLE people on what they prefer, so far noone has taken an interest in my patch.

[Sérgio Martins]

> [13:30] <tronical> milian: I suppose an implicit inclusion of string.h from
> somewhere, so that the extern "C" variant of strlen is chosen?

string.h isn't being included. What happens is that some compilers, when including the cstring, cstdlib etc,  will also define the stuff in global namespace.

QNX's compiler does'n do this, but it isn't a compiler bug, as the standard doesn't specify: http://stackoverflow.com/questions/9149368/is-it-ok-to-qualify-c-functions-with-the-std-namespace


> > Wouldn't a simpler way to fix this would be to add one or more includes of <string.h>?
> 
> Millian, did you see Darin's comment? If that works then it would seem like a much simpler solution :)

Build also fails on malloc(), we would need stdlib.h too, we're opening a can of worms.

IMO, duplicating C and C++ headers is not a good solution.
Besides, C header usage in C++ is discouraged by the standard.

A couple of "using" or std:: would be best.

[Milian Wolff]

Created attachment 174978 [details]
Patch

Updated patch. I'll still try to resolve this issue somehow in ANGLE upstream...

[Milian Wolff]

Upstream ANGLE diverged quite a bit, I've submitted a patch but I'm not sure whether that is all that is required to get it working for WebKit. See: https://codereview.appspot.com/6843083/

[Milian Wolff]

Ok, my patch was accepted upstream in ANGLE. What do I do now to get this issue resolved in WebKit? Do I have to submit a patch that updates the local checkout of ANGLE in the WebKit source tree?

See: https://codereview.appspot.com/6843083/

Considering that upstream ANGLE diverged, we cannot apply the patch from there 1to1 to the WebKit copy.

[Andras Becsi]

I've seen patches against the copy of ANGLE in WebKit, but I'm not sure that's the right way.

CC'ing a couple of people who might know how updates are done.

[Kenneth Russell]

@dino did the last major update of the copy of ANGLE in WebKit.

[Dean Jackson]

(In reply to comment #16)
> @dino did the last major update of the copy of ANGLE in WebKit.

WebKit ANGLE diverged from upstream ANGLE, but maybe we can sync back up now. I'll look into this.

[Dean Jackson]

(In reply to comment #17)
> (In reply to comment #16)
> > @dino did the last major update of the copy of ANGLE in WebKit.
> 
> WebKit ANGLE diverged from upstream ANGLE, but maybe we can sync back up now. I'll look into this.

If this is urgent, and it is already landed in upstream, then you can apply to the WebKit fork.

The goal is to get back onto the official ANGLE as soon as possible, so that we can avoid these situations.

[Brent Fulgham]

The QT port is no longer part of WebKit.