CheckedInt.h Intel C++ compilation issue

RESOLVED FIXED in mozilla19

Status

()

Core
MFBT
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Alex Kontos, Assigned: bjacob)

Tracking

({64bit})

15 Branch
mozilla19
x86_64
Windows 7
64bit
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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.
(Reporter)

Comment 1

5 years ago
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?
(Reporter)

Updated

5 years ago
Hardware: x86 → x86_64
(Reporter)

Updated

5 years ago
Keywords: 64bit

Updated

5 years ago
Component: Untriaged → Build Config
Component: Build Config → XPCOM
Product: Firefox → Core
(Assignee)

Comment 2

5 years ago
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.
(Reporter)

Comment 3

5 years ago
Created attachment 653874 [details]
Compilation Log
Attachment #653725 - Attachment is obsolete: true
(Reporter)

Comment 4

5 years ago
(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).
(Assignee)

Comment 5

5 years ago
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.
(Reporter)

Comment 6

5 years ago
(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.
(Reporter)

Updated

5 years ago
Status: UNCONFIRMED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WORKSFORME
(Assignee)

Comment 7

5 years ago
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 → ---
(Assignee)

Comment 8

5 years ago
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)

Updated

5 years ago
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/integration/mozilla-inbound/rev/c9c99a2f189c
Target Milestone: --- → mozilla19
https://hg.mozilla.org/mozilla-central/rev/c9c99a2f189c
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago5 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.