Closed Bug 784309 Opened 7 years ago Closed 7 years ago

CheckedInt.h Intel C++ compilation issue

Categories

(Core :: MFBT, defect)

15 Branch
x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19

People

(Reporter: alexboy94, Assigned: bjacob)

Details

(Keywords: 64bit)

Attachments

(2 files, 1 obsolete file)

Attached image 45742.png (obsolete) —
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.
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?
Hardware: x86 → x86_64
Keywords: 64bit
Component: Untriaged → Build Config
Component: Build Config → XPCOM
Product: Firefox → Core
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.
Attached file Compilation Log
Attachment #653725 - Attachment is obsolete: true
(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).
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.
(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.
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
WORKSFORME isn't what you wanted here ;-)

We'll land that patch then. Thanks for reporting to Intel.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: WORKSFORME → ---
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.
Attachment #653882 - Flags: review?(Ms2ger)
Assignee: nobody → bjacob
Status: REOPENED → ASSIGNED
Component: XPCOM → MFBT
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
Attachment #653882 - Flags: review?(Ms2ger) → review+
Any reason this hasn't landed?
I tend to forget to land patches. sorry about that.
https://hg.mozilla.org/mozilla-central/rev/c9c99a2f189c
Status: ASSIGNED → RESOLVED
Closed: 7 years ago7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.