Bug 55221 - Build fix for Intel ICC Compiler
Summary: Build fix for Intel ICC Compiler
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-25 08:31 PST by Alexis Menard (darktears)
Modified: 2011-02-28 12:25 PST (History)
5 users (show)

See Also:


Attachments
Proposed patch to fix the build. (1.24 KB, patch)
2011-02-25 08:31 PST, Alexis Menard (darktears)
darin: review+
darin: commit-queue+
Details | Formatted Diff | Diff
V2 with a comment. (1.31 KB, patch)
2011-02-25 10:27 PST, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
New patch with Alexey's comments (2.05 KB, patch)
2011-02-28 05:11 PST, Alexis Menard (darktears)
ap: review+
Details | Formatted Diff | Diff
New patch V2 with Alexey's comments (2.07 KB, patch)
2011-02-28 09:03 PST, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexis Menard (darktears) 2011-02-25 08:31:10 PST
With Intel ICC compiler for Linux 64 bits, there is some issue to link against a version of WebKit build with ICC : undefined reference to the destructor of SVGTransformable(). I think ICC is confused with the virtual inheritance and virtual methods in the class.
Comment 1 Alexis Menard (darktears) 2011-02-25 08:31:40 PST
Created attachment 83817 [details]
Proposed patch to fix the build.
Comment 2 Darin Adler 2011-02-25 10:11:49 PST
Comment on attachment 83817 [details]
Proposed patch to fix the build.

Might have been slightly better to add a brief comment as well. Otherwise I see no reason someone wouldn’t just remove this later.
Comment 3 Alexis Menard (darktears) 2011-02-25 10:27:44 PST
Created attachment 83832 [details]
V2 with a comment.
Comment 4 Alexey Proskuryakov 2011-02-25 21:39:50 PST
This is surprising. I thought that Intel ICC was supposed to be an extremely compliant compiler, so perhaps there is a real reason why an implementation is needed? I can't think of any though.

I suggest adding the implementation in a .cpp file if that works with ICC, because an inline virtual destructor in a base class causes significant code bloat.

Please mark a patch for review, so that it doesn't get overlooked.
Comment 5 Alexis Menard (darktears) 2011-02-28 05:11:20 PST
Created attachment 84041 [details]
New patch with Alexey's comments

@Alexey : Me too, actually ICC is a very good compiler. Though his main problem is that it's not widely used therefore there is still bugs. For instance there is a huge gap from version 11 to version 12 in term of bugfixing (in a good way). I think if I have a bit of time I will try to reproduce in a small C++ example and fill a bug to Intel. Thanks for the review.
Comment 6 Alexey Proskuryakov 2011-02-28 08:53:00 PST
Comment on attachment 84041 [details]
New patch with Alexey's comments

+{
+
+}

We don't put a blank line in empty function bodies.

+    // This destructor is needed in order to link correctly with Intel ICC.

The comment is only needed to prevent accidental removal of the destructor - it would be sufficient (and better) to only have it in .cpp file. People looking for something else in the header don't need to know about this.
Comment 7 Alexis Menard (darktears) 2011-02-28 09:03:36 PST
Created attachment 84070 [details]
New patch V2 with Alexey's comments
Comment 8 Alexey Proskuryakov 2011-02-28 09:04:55 PST
Comment on attachment 84070 [details]
New patch V2 with Alexey's comments

Thank you!
Comment 9 WebKit Commit Bot 2011-02-28 10:21:28 PST
Comment on attachment 84070 [details]
New patch V2 with Alexey's comments

Rejecting attachment 84070 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sf', 'bu..." exit_code: 2

Last 500 characters of output:
ccessibility/AccessibilityMenuList.cpp -o /Projects/CommitQueue/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/AccessibilityMenuList.o

** BUILD FAILED **


The following build commands failed:
WebCore:
	CompileC /Projects/CommitQueue/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/AccessibilityMenuListOption.o /Projects/CommitQueue/Source/WebCore/accessibility/AccessibilityMenuListOption.cpp normal x86_64 c++ com.apple.compilers.gcc.4_2
(1 failure)


Full output: http://queues.webkit.org/results/8074328
Comment 10 Alexey Proskuryakov 2011-02-28 10:28:41 PST
Comment on attachment 84070 [details]
New patch V2 with Alexey's comments

Trying again...
Comment 11 Eric Seidel (no email) 2011-02-28 10:37:19 PST
Odd.  The commit-queue hasn't historically had flaky builds in the past.  I suspect this will fail again, but it's worth the try!
Comment 12 WebKit Commit Bot 2011-02-28 12:23:19 PST
The commit-queue encountered the following flaky tests while processing attachment 84070 [details]:

http/tests/websocket/tests/workers/close-in-shared-worker.html bug 55397 (author: abarth@webkit.org)
The commit-queue is continuing to process your patch.
Comment 13 WebKit Commit Bot 2011-02-28 12:25:19 PST
Comment on attachment 84070 [details]
New patch V2 with Alexey's comments

Clearing flags on attachment: 84070

Committed r79896: <http://trac.webkit.org/changeset/79896>
Comment 14 WebKit Commit Bot 2011-02-28 12:25:25 PST
All reviewed patches have been landed.  Closing bug.