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 | Splinter Review

Description User image 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 User image 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 User image 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 User image Alex Kontos 2012-08-21 11:41:26 PDT
Created attachment 653874 [details]
Compilation Log
Comment 4 User image 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 User image 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 User image 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 User image 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 User image 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 User image :Ms2ger (⌚ UTC+1/+2) 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 User image Jeff Walden [:Waldo] (remove +bmo to email) 2012-11-01 23:22:50 PDT
Any reason this hasn't landed?
Comment 11 User image Benoit Jacob [:bjacob] (mostly away) 2012-11-02 08:28:43 PDT
I tend to forget to land patches. sorry about that.
Comment 12 User image Benoit Jacob [:bjacob] (mostly away) 2012-11-02 08:31:15 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9c99a2f189c
Comment 13 User image 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.