Bug 34804 - Some WTF headers fail to compile under VS2008 and newer
Summary: Some WTF headers fail to compile under VS2008 and newer
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar, PlatformOnly
Depends on:
Blocks: 53445
  Show dependency treegraph
 
Reported: 2010-02-10 09:49 PST by Adam Roben (:aroben)
Modified: 2022-02-11 13:59 PST (History)
5 users (show)

See Also:


Attachments
Patch proposed for removing inline statement in order to compile under VS2008 (1.24 KB, patch)
2013-02-15 07:05 PST, Victor Bonnedun
no flags Details | Formatted Diff | Diff
Patch proposed for removing inline statement in order to compile under VS2008 (updated) (2.87 KB, patch)
2013-02-15 08:08 PST, Victor Bonnedun
darin: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Roben (:aroben) 2010-02-10 09:49:21 PST
PassRefPtr.h fails to compile under VS2010 RC. This line:

friend PassRefPtr adoptRef<T>(T*);

generates a C4396 error: "the inline specifier cannot be used when a friend declaration refers to a specialization of a function template". See http://msdn.microsoft.com/en-us/library/bb384968.aspx for more details.
Comment 1 Adam Roben (:aroben) 2010-02-10 09:50:07 PST
Sorry, that's a warning, not an error.

We should probably just disable this warning in common.vsprops.
Comment 2 Adam Roben (:aroben) 2010-02-10 09:53:23 PST
I think this warning also affects VS2008.
Comment 3 Eugene Lavrenov 2010-02-10 21:14:36 PST
Hi, I just submitted a patch for this. See bug 34828. There are actually three files in JavaScriptCore project, which are affected. I managed to build it with VS2008. Safari works fine.
Comment 4 Eugene Lavrenov 2010-02-10 21:51:52 PST
I do not think you should disable this warning in common.vsprops because accordingly to Microsoft: "The compiler issues warning C4396 and IGNORES THE INLINE SPECIFIER." http://msdn.microsoft.com/en-us/library/bb384968.aspx
I am pretty sure that VS2005 compiler as well as many others also ignored inline specifier in that particular case, but without warning. :)
Comment 5 Adam Roben (:aroben) 2010-02-11 07:23:43 PST
*** Bug 34828 has been marked as a duplicate of this bug. ***
Comment 6 Adam Roben (:aroben) 2010-02-11 07:25:01 PST
As described in bug 34828, this affects a few other WTF headers.
Comment 7 Adam Roben (:aroben) 2010-07-26 09:51:02 PDT
<rdar://problem/8234558>
Comment 8 Adam Roben (:aroben) 2010-07-26 10:38:20 PDT
(In reply to comment #4)
> I do not think you should disable this warning in common.vsprops because accordingly to Microsoft: "The compiler issues warning C4396 and IGNORES THE INLINE SPECIFIER." http://msdn.microsoft.com/en-us/library/bb384968.aspx
> I am pretty sure that VS2005 compiler as well as many others also ignored inline specifier in that particular case, but without warning. :)

I think the way to make a decision here is roughly:

* Does any compiler honor the inline specifier on adoptRef?
   * No: we should just remove it
   * Yes:
      * Does removing the inline specifier have any negative effects for those compilers which honor it?
         * No: we should just remove it
         * Yes: we should ignore the warning
Comment 9 Victor Bonnedun 2013-02-15 07:05:20 PST
Created attachment 188561 [details]
Patch proposed for removing inline statement in order to compile under VS2008
Comment 10 Victor Bonnedun 2013-02-15 07:09:08 PST
(In reply to comment #8)
> (In reply to comment #4)
> > I do not think you should disable this warning in common.vsprops because accordingly to Microsoft: "The compiler issues warning C4396 and IGNORES THE INLINE SPECIFIER." http://msdn.microsoft.com/en-us/library/bb384968.aspx
> > I am pretty sure that VS2005 compiler as well as many others also ignored inline specifier in that particular case, but without warning. :)
> 
> I think the way to make a decision here is roughly:
> 
> * Does any compiler honor the inline specifier on adoptRef?
>    * No: we should just remove it
>    * Yes:
>       * Does removing the inline specifier have any negative effects for those compilers which honor it?
>          * No: we should just remove it
>          * Yes: we should ignore the warning

Hello,

Any news about this problem ?

I am currently trying to compile webkit using VS2008 and I encountered this issue.
As mentionned in Bug 34828, there are 3 inline statements that should be removed in order to compile under VS2008 (I submitted a patch).
Comment 11 Victor Bonnedun 2013-02-15 08:08:32 PST
Created attachment 188575 [details]
Patch proposed for removing inline statement in order to compile under VS2008 (updated)
Comment 12 Darin Adler 2013-02-15 09:44:56 PST
Comment on attachment 188575 [details]
Patch proposed for removing inline statement in order to compile under VS2008 (updated)

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

Patch is incorrect, at least for non-Microsoft compilers, has no change log, and also does not apply successfully.

> JavaScriptCore/heap/PassWeak.h:162
> -template<typename T> PassWeak<T> inline adoptWeak(WeakImpl* impl)
> +template<typename T> PassWeak<T> adoptWeak(WeakImpl* impl)

This is an unacceptable change for other compilers. The inline keyword tells the compiler to inline this function. Not sure why Microsoft’s compiler is having a problem with this, but please don’t punish all others for this mistake.
Comment 13 Darin Adler 2013-02-15 09:46:34 PST
Sorry, I see now that others suggest that perhaps all the compilers are ignoring this inline unless inline was specified in the friend declaration. To make this change we need to get data on that. Adam Roben described the research that needs to be done. If someone does that research then we can make the appropriate change.