Bug 39945 - Compilation error with clang in JSDOMBinding.h
Summary: Compilation error with clang in JSDOMBinding.h
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-31 02:12 PDT by Olivier Goffart
Modified: 2010-05-31 15:16 PDT (History)
5 users (show)

See Also:


Attachments
Patch (53 bytes, text/plain)
2010-05-31 02:19 PDT, Olivier Goffart
no flags Details
Patch (1.55 KB, patch)
2010-05-31 02:21 PDT, Olivier Goffart
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Olivier Goffart 2010-05-31 02:12:47 PDT
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
Comment 1 Olivier Goffart 2010-05-31 02:19:14 PDT
Created attachment 57441 [details]
Patch
Comment 2 Olivier Goffart 2010-05-31 02:21:00 PDT
Created attachment 57442 [details]
Patch
Comment 3 Benjamin Poulain 2010-05-31 02:25:50 PDT
Adding Oliver, he can probably review the patch.
Comment 4 Oliver Hunt 2010-05-31 09:58:42 PDT
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)
Comment 5 Darin Adler 2010-05-31 10:10:54 PDT
(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.
Comment 6 Oliver Hunt 2010-05-31 10:49:53 PDT
Comment on attachment 57442 [details]
Patch

r=me as well then
Comment 7 WebKit Commit Bot 2010-05-31 15:16:37 PDT
Comment on attachment 57442 [details]
Patch

Clearing flags on attachment: 57442

Committed r60451: <http://trac.webkit.org/changeset/60451>
Comment 8 WebKit Commit Bot 2010-05-31 15:16:44 PDT
All reviewed patches have been landed.  Closing bug.