Bug 22299 – Improve visual studio 2008 sp1 compilation

• Component: Tools / Tests
• Status: RESOLVED WONTFIX

[Marc-Antoine Ruel]

Fixes some compilation issues with Visual Studio 2008 Service Pack 1.

There is still issues with the link step. At least, the compile step now pass.

[Marc-Antoine Ruel]

Created attachment 25203 [details]
Template fixes

VS2008 refuses to have a specialization of a function template as a friend. It can only mark as a friend the unspecialized template.

To fix this, a different typename must be used for the function template description. I just added an underscore to the typenames.

[Marc-Antoine Ruel]

This patch builds and run on Mac OS X 10.5.5 too.

[Eric Seidel (no email)]

Comment on attachment 25203 [details]
Template fixes

You should set CHANGE_LOG_NAME or REAL_NAME to "Marc-Antoine Ruel" (or just edit the ChangeLog after the fact to have your real name instead of maruel).

Why add _ to the end of these template parameter names?  That's not WebKit style, it might be google style, I'm not even sure of that (but that doesn't really have any bearing here).

Also generally we mention the bug number in the changelog, and it would be nice for documentations sake to have the text of the warning/error in the bug or the changelog.  Maciej or Darin (Adler) would be most likely to review this change, as they're the two template experts in the WebKit community (that I know of).

r- for the reasons mentioned above.

[Eric Seidel (no email)]

Pasting the error is not really necessary in the bug.  You've explained the bug nicely above:

VS2008 refuses to have a specialization of a function template as a friend. It
can only mark as a friend the unspecialized template.

To fix this, a different typename must be used for the function template
description.

But you should add that (or similar) to the ChangeLog, since the ChangeLog becomes the commit log and is the most accessible record.

[Eric Seidel (no email)]

CCing our template gurus.

[Marc-Antoine Ruel]

Created attachment 25225 [details]
Fixes compilation issues with VS2008

Same as previous with ChangeLog updated.

[Sam Weinig]

Comment on attachment 25225 [details]
Fixes compilation issues with VS2008

We don't, nor do we plan to in the short term, support Visual Studio 2008.  Therefore, I don't really see any reason to add these changes.

[Eric Seidel (no email)]

Comment on attachment 25225 [details]
Fixes compilation issues with VS2008

Marc and I talked this out over IM.  The change looks fine.  I'm still slightly confused as to why the type names need to be different in the forward declaration.

He pointed out that:
http://msdn.microsoft.com/en-us/library/bb384871.aspx
documents this error.

I would like either Maciej or Darin (Adler) to glance at this before one of us lands it though.

[Sam Weinig]

Comment on attachment 25225 [details]
Fixes compilation issues with VS2008

Based on popular demand, I am reverting my r-.  I still don't think there is a compelling reason to take this patch if we don't plan on officially supporting Visual Studio 2008, but I will leave the review status to wiser people than I.

[Marc-Antoine Ruel]

Ok, after the back and forth, I decided to verify if this fix was indeed for 3772 (error) or 4396 (warning).  I had made the patch a few months ago so I had assumed I was fixing an error. After testing, I just realized this patch fixes 4396.

So don't bother anymore. :(