Bug 39945 – Compilation error with clang in JSDOMBinding.h

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Olivier Goffart]

webkit/WebCore/bindings/js/JSDOMBinding.h:229:20: error: no viable conversion from 'WebCore::JSNode *' to 'JSC::JSValue'

webkit/JavaScriptCore/runtime/JSValue.h:807:21: note: candidate constructor not viable: cannot convert argument of incomplete type 'WebCore::JSNode *' to 'JSC::JSCell *'
    inline JSValue::JSValue(JSCell* ptr)



     JSNode is only forward declared at this point. And since neither 
     "wrapper" nor JSValue are type-dependent. Compilers should report errors 
     at the first compilation pass. 
      
     The fix is to move the conversion the line above, as the call to the 
     function getCachedDOMNodeWrapper is type-dependent, the conversion will 
     happen at template-instantiation time. 
      
     See also http://llvm.org/bugs/show_bug.cgi?id=7244

[Olivier Goffart]

Created attachment 57441 [details]
Patch

[Olivier Goffart]

Created attachment 57442 [details]
Patch

[Benjamin Poulain]

Adding Oliver, he can probably review the patch.

[Oliver Hunt]

Comment on attachment 57442 [details]
Patch

Why doesn't this fail in gcc or msvc? (i'm removing the r+ to prevent commit bot shenanigans till i understand thwe issue)

[Darin Adler]

(In reply to comment #4)
> Why doesn't this fail in gcc or msvc? (i'm removing the r+ to prevent commit bot shenanigans till i understand thwe issue)

Here’s my understanding: The older compilers don't implement as much checking at template-definition time as clang does. These older C++ compilers tended to treat templates more like macros and not do much checking on them until the template was instantiated. But the C++ standard allows (perhaps requires) additional checking at the time the template is defined.

On the other hand, I was led to believe that some folks have successfully compiled this code with at least some version of clang, so this may simply be a mistake in a certain version of clang rather than correct behavior.

[Oliver Hunt]

Comment on attachment 57442 [details]
Patch

r=me as well then

[WebKit Commit Bot]

Comment on attachment 57442 [details]
Patch

Clearing flags on attachment: 57442

Committed r60451: <http://trac.webkit.org/changeset/60451>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.