Last Comment Bug 784309 - CheckedInt.h Intel C++ compilation issue
: CheckedInt.h Intel C++ compilation issue
Status: RESOLVED FIXED
: 64bit
Product: Core
Classification: Components
Component: MFBT (show other bugs)
: 15 Branch
: x86_64 Windows 7
: -- normal (vote)
: mozilla19
Assigned To: Benoit Jacob [:bjacob] (mostly away)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-21 05:29 PDT by Alex Kontos
Modified: 2012-11-02 17:23 PDT (History)
4 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
45742.png (118.52 KB, image/png)
2012-08-21 05:29 PDT, Alex Kontos
no flags Details
Compilation Log (2.82 MB, text/plain)
2012-08-21 11:41 PDT, Alex Kontos
no flags Details
Rename IsSigned template paramter to IsTSigned to avoid conflict with struct name (1.60 KB, patch)
2012-08-21 12:03 PDT, Benoit Jacob [:bjacob] (mostly away)
Ms2ger: review+
Details | Diff | Review

Description Alex Kontos 2012-08-21 05:29:06 PDT
Created attachment 653725 [details]
45742.png

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:14.0) Gecko/20120819 Firefox/14.0.1
Build ID: 20120819213618

Steps to reproduce:

Compiled Firefox 15 with Intel's C++ Compiler.


Actual results:

Compilation got stuck at dom_quickstubs.cpp, with CheckedInt.h producing errors such as typename is not allowed.
Screenshot attached.


Expected results:

Compilation should have proceeded as normal. Attempted to have dom_quickstubs.cpp compiled by cl and the rest by icl but to no avail.
Comment 1 Alex Kontos 2012-08-21 05:31:00 PDT
I'm thinking this could be a bug with the compiler, but Firefox 14.0.1 compiled without issue. Where any changes made to SpiderMonkey?
Comment 2 Benoit Jacob [:bjacob] (mostly away) 2012-08-21 11:01:43 PDT
There definitely were changes in CheckedInt around this timeframe. This code compiles without warnings in GCC, Clang and MSVC. I don't understand the warning shown in your screenshot.
Comment 3 Alex Kontos 2012-08-21 11:41:26 PDT
Created attachment 653874 [details]
Compilation Log
Comment 4 Alex Kontos 2012-08-21 11:42:58 PDT
(In reply to Alex Kontos from comment #3)
> Created attachment 653874 [details]
> Compilation Log

I've attached the compilation log and used make. (As opposed to pymake in the screenshot).
Comment 5 Benoit Jacob [:bjacob] (mostly away) 2012-08-21 12:03:17 PDT
Created attachment 653882 [details] [diff] [review]
Rename IsSigned template paramter to IsTSigned to avoid conflict with struct name

Can you please try this patch? This seems to be a compiler bug, but a fair one as I am using the same identifier to mean two different things.
Comment 6 Alex Kontos 2012-08-21 14:29:19 PDT
(In reply to Benoit Jacob [:bjacob] from comment #5)
> Created attachment 653882 [details] [diff] [review]
> Rename IsSigned template paramter to IsTSigned to avoid conflict with struct
> name
> 
> Can you please try this patch? This seems to be a compiler bug, but a fair
> one as I am using the same identifier to mean two different things.

The patch works. I'll report the bug to Intel.
Comment 7 Benoit Jacob [:bjacob] (mostly away) 2012-08-21 14:35:47 PDT
WORKSFORME isn't what you wanted here ;-)

We'll land that patch then. Thanks for reporting to Intel.
Comment 8 Benoit Jacob [:bjacob] (mostly away) 2012-08-21 14:37:37 PDT
Comment on attachment 653882 [details] [diff] [review]
Rename IsSigned template paramter to IsTSigned to avoid conflict with struct name

This patch is trivial enough and even improves the code as it was not great to use IsSigned for two different things.
Comment 9 :Ms2ger 2012-08-22 00:53:55 PDT
Comment on attachment 653882 [details] [diff] [review]
Rename IsSigned template paramter to IsTSigned to avoid conflict with struct name

Review of attachment 653882 [details] [diff] [review]:
-----------------------------------------------------------------

r=me
Comment 10 Jeff Walden [:Waldo] (remove +bmo to email) 2012-11-01 23:22:50 PDT
Any reason this hasn't landed?
Comment 11 Benoit Jacob [:bjacob] (mostly away) 2012-11-02 08:28:43 PDT
I tend to forget to land patches. sorry about that.
Comment 12 Benoit Jacob [:bjacob] (mostly away) 2012-11-02 08:31:15 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9c99a2f189c
Comment 13 Ryan VanderMeulen [:RyanVM] 2012-11-02 17:23:10 PDT
https://hg.mozilla.org/mozilla-central/rev/c9c99a2f189c

Note You need to log in before you can comment on or make changes to this bug.