Bug 59261

Summary: Compile error with GCC 4.6.0, tries to assign unsigned& to bitfield
Product: WebKit Reporter: Xan Lopez <xan.lopez>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, menard, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
unaryplus.diff
none
Patch for landing andersca: review+

Xan Lopez
Reported 2011-04-22 17:36:13 PDT
When compiling JSC with GCC 4.6.0: CXX Source/JavaScriptCore/API/libJavaScriptCore_la-JSBase.lo In file included from ../../Source/JavaScriptCore/runtime/ScopeChain.h:25:0, from ../../Source/JavaScriptCore/runtime/JSObject.h:35, from ../../Source/JavaScriptCore/runtime/JSArray.h:24, from ../../Source/JavaScriptCore/runtime/JSGlobalObject.h:25, from ../../Source/JavaScriptCore/runtime/JSObjectWithGlobalObject.h:29, from ../../Source/JavaScriptCore/runtime/JSFunction.h:27, from ../../Source/JavaScriptCore/runtime/Executable.h:30, from ../../Source/JavaScriptCore/bytecode/EvalCodeCache.h:32, from ../../Source/JavaScriptCore/bytecode/CodeBlock.h:33, from ../../Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:33, from ../../Source/JavaScriptCore/jsc.cpp:25: ../../Source/JavaScriptCore/runtime/Structure.h: In static member function ‘static JSC::StructureTransitionTable::Hash::Key JSC::StructureTransitionTable::keyForWeakGCMapFinalizer(void*, JSC::Structure*)’: ../../Source/JavaScriptCore/runtime/Structure.h:301:94: error: cannot bind bitfield ‘structure->JSC::Structure::m_attributesInPrevious’ to ‘unsigned int&’ the reason for this is explained in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45253, but basically comes down to GCC not being able to fully figure the type of the parameter and defaulting to 'unsigned&', which cannot be assigned to a bitfield. As this seems to be technically correct, and GCC is already doing it, the workaround suggested in the upstream bug report is to use unary '+' to force a proper type detection. The attached patch does this.
Attachments
unaryplus.diff (4.77 KB, patch)
2011-04-22 17:41 PDT, Xan Lopez
no flags
Patch for landing (4.15 KB, patch)
2011-05-03 06:21 PDT, Alexis Menard (darktears)
andersca: review+
Xan Lopez
Comment 1 2011-04-22 17:41:53 PDT
Created attachment 90814 [details] unaryplus.diff
Alexey Proskuryakov
Comment 2 2011-04-22 21:52:14 PDT
Comment on attachment 90814 [details] unaryplus.diff View in context: https://bugs.webkit.org/attachment.cgi?id=90814&action=review > Source/JavaScriptCore/runtime/Structure.cpp:103 > + // Use unary '+' for m_attributesInPrevious to force precise > + // type detection with GCC >= 4.6.0. See > + // https://bugs.webkit.org/show_bug.cgi?id=59261 for more > + // details. Can this be written in one line? > Source/JavaScriptCore/runtime/Structure.cpp:132 > + // Use unary '+' for m_attributesInPrevious to force precise > + // type detection with GCC >= 4.6.0. See > + // https://bugs.webkit.org/show_bug.cgi?id=59261 for more > + // details. Ditto. > Source/JavaScriptCore/runtime/Structure.h:304 > + // Use unary '+' for m_attributesInPrevious to force precise > + // type detection with GCC >= 4.6.0. See > + // https://bugs.webkit.org/show_bug.cgi?id=59261 for more > + // details. Ditto.
Xan Lopez
Comment 3 2011-04-25 16:20:47 PDT
(In reply to comment #2) > (From update of attachment 90814 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=90814&action=review > > > Source/JavaScriptCore/runtime/Structure.cpp:103 > > + // Use unary '+' for m_attributesInPrevious to force precise > > + // type detection with GCC >= 4.6.0. See > > + // https://bugs.webkit.org/show_bug.cgi?id=59261 for more > > + // details. > > Can this be written in one line? > Wouldn't that be a lot more difficult to read? No other comment in the file seems to do this when they are long-ish. Are you asking because of the weird line-breaks and the bugzilla URL?
Eric Seidel (no email)
Comment 4 2011-04-26 15:20:23 PDT
@ap, please r+ or r-.
Alexey Proskuryakov
Comment 5 2011-04-26 15:23:59 PDT
static_cast would be my preference, but let's see what Anders or Darin think.
Anders Carlsson
Comment 6 2011-04-26 15:38:12 PDT
Comment on attachment 90814 [details] unaryplus.diff The patch looks correct, but I'd prefer a more generic comment; you'll get this error with any compiler whose standard library has an std::make_pair implementation that takes value references. How about something like: // Newer versions of the STL have an std::make_pair function that takes rvalue references. // When either of the parameters are bitfields, the C++ compiler will try to bind them as lvalues, which is invalid. To work around this, use unary "+" to make the parameter an rvalue. // See https://bugs.webkit.org/show_bug.cgi?id=59261 for more details.
Alexis Menard (darktears)
Comment 7 2011-05-03 06:21:47 PDT
Created attachment 92066 [details] Patch for landing
Alexis Menard (darktears)
Comment 8 2011-05-03 10:34:34 PDT
Note You need to log in before you can comment on or make changes to this bug.