Local build error with MSVC 2012 in AccessibleWrap.cpp(924) : error C2440: '=' : cannot convert from 'mozilla::a11y::RelationType' to 'int32_t' / This conversion requires an explicit cast (static_cast, C-style cast or function-style cast)

RESOLVED FIXED in mozilla27

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla27
x86_64
Windows NT
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
Build error when building mozilla-inbound on Windows (with Visual Studio 2012 Update 3):
{
/mozilla-central/accessible/src/windows/msaa/AccessibleWrap.cpp(924) : error C2440: '=' : cannot convert from 'mozilla::a11y::RelationType' to 'int32_t'
         This conversion requires an explicit cast (static_cast, C-style cast or function-style cast)
/mozilla-central/accessible/src/windows/msaa/AccessibleWrap.cpp(927) : error C2440: '=' : cannot convert from 'mozilla::a11y::RelationType' to 'int32_t'
         This conversion requires an explicit cast (static_cast, C-style cast or function-style cast)
/mozilla-central/accessible/src/windows/msaa/AccessibleWrap.cpp(930) : error C2440: '=' : cannot convert from 'mozilla::a11y::RelationType' to 'int32_t'
         This conversion requires an explicit cast (static_cast, C-style cast or function-style cast)
/mozilla-central/accessible/src/windows/msaa/AccessibleWrap.cpp(933) : error C2440: '=' : cannot convert from 'mozilla::a11y::RelationType' to 'int32_t'
         This conversion requires an explicit cast (static_cast, C-style cast or function-style cast)
/mozilla-central/accessible/src/windows/msaa/AccessibleWrap.cpp(936) : error C2440: '=' : cannot convert from 'mozilla::a11y::RelationType' to 'int32_t'
         This conversion requires an explicit cast (static_cast, C-style cast or function-style cast)
/mozilla-central/accessible/src/windows/msaa/AccessibleWrap.cpp(939) : error C2440: '=' : cannot convert from 'mozilla::a11y::RelationType' to 'int32_t'
         This conversion requires an explicit cast (static_cast, C-style cast or function-style cast)
/mozilla-central/accessible/src/windows/msaa/AccessibleWrap.cpp(942) : error C2440: '=' : cannot convert from 'mozilla::a11y::RelationType' to 'int32_t'
         This conversion requires an explicit cast (static_cast, C-style cast or function-style cast)
/mozilla-central/accessible/src/windows/msaa/AccessibleWrap.cpp(945) : error C2440: '=' : cannot convert from 'mozilla::a11y::RelationType' to 'int32_t'
         This conversion requires an explicit cast (static_cast, C-style cast or function-style cast)
/mozilla-central/accessible/src/windows/msaa/AccessibleWrap.cpp(948) : error C2440: '=' : cannot convert from 'mozilla::a11y::RelationType' to 'int32_t'
         This conversion requires an explicit cast (static_cast, C-style cast or function-style cast)
/mozilla-central/accessible/src/windows/msaa/AccessibleWrap.cpp(951) : error C2440: '=' : cannot convert from 'mozilla::a11y::RelationType' to 'int32_t'
         This conversion requires an explicit cast (static_cast, C-style cast or function-style cast)
/mozilla-central/accessible/src/windows/msaa/AccessibleWrap.cpp(954) : error C2440: '=' : cannot convert from 'mozilla::a11y::RelationType' to 'int32_t'
         This conversion requires an explicit cast (static_cast, C-style cast or function-style cast)
/mozilla-central/accessible/src/windows/msaa/AccessibleWrap.cpp(957) : error C2440: '=' : cannot convert from 'mozilla::a11y::RelationType' to 'int32_t'
         This conversion requires an explicit cast (static_cast, C-style cast or function-style cast)
/mozilla-central/accessible/src/windows/msaa/AccessibleWrap.cpp(960) : error C2440: '=' : cannot convert from 'mozilla::a11y::RelationType' to 'int32_t'
         This conversion requires an explicit cast (static_cast, C-style cast or function-style cast)
/mozilla-central/accessible/src/windows/msaa/AccessibleWrap.cpp(963) : error C2440: '=' : cannot convert from 'mozilla::a11y::RelationType' to 'int32_t'
         This conversion requires an explicit cast (static_cast, C-style cast or function-style cast)
/mozilla-central/accessible/src/windows/msaa/AccessibleWrap.cpp(966) : error C2440: '=' : cannot convert from 'mozilla::a11y::RelationType' to 'int32_t'
         This conversion requires an explicit cast (static_cast, C-style cast or function-style cast)
/mozilla-central/accessible/src/windows/msaa/AccessibleWrap.cpp(969) : error C2440: '=' : cannot convert from 'mozilla::a11y::RelationType' to 'int32_t'
         This conversion requires an explicit cast (static_cast, C-style cast or function-style cast)
/mozilla-central/accessible/src/windows/msaa/AccessibleWrap.cpp(972) : error C2440: '=' : cannot convert from 'mozilla::a11y::RelationType' to 'int32_t'
         This conversion requires an explicit cast (static_cast, C-style cast or function-style cast)
}

Looks like this class was recently added via MOZ_BEGIN_ENUM_CLASS(RelationType) in bug 748639:
http://hg.mozilla.org/integration/mozilla-inbound/rev/e8f7b915ad46#l3.19
...and we're assigning values of this type into "xpRelation", which is what's triggering the error:
http://hg.mozilla.org/integration/mozilla-inbound/rev/e8f7b915ad46#l21.11
...because xpRelation is declared as an integer:
http://hg.mozilla.org/integration/mozilla-inbound/annotate/e8f7b915ad46/accessible/src/windows/msaa/AccessibleWrap.cpp#l899

Presumably xpRelation's type should be changed.
(Assignee)

Comment 1

5 years ago
It's absolutely correct that the compiler's erroring out on this, BTW - enum class types are supposed to be type-safe, so they can't be implicitly converted to int.

(I assume it only avoided blowing up on TBPL because (a) this is Windows-only code and (b) the compiler we use on Windows TBPL builders must be old enough to get the fallback (non-typesafe) path for MOZ_BEGIN_ENUM_CLASS -- i.e. it just translates to enum { ... }. or something like that.)
(Assignee)

Comment 2

5 years ago
Created attachment 819405 [details] [diff] [review]
fix v1: just static_cast in the assignments

(In reply to Daniel Holbert [:dholbert] from comment #0)
> Presumably xpRelation's type should be changed.

Actually, it's not quite that simple; we initialize xpRelation as -1 (which is out of bounds for RelationType), so we can't change it unless we add a suitable "INVALID" value to its enum list that we could use in place of -1 here.

I don't know that it's worth adding such a value just for this one use case, though.  There are other ways we could restructure this code to not depend on -1, but that'd involve some refactoring which I don't really want to do.

So, here's the low-impact solution -- keep xpRelation an int32_t, and static_cast all of the enum values to its type when we do assignments.  It's ugly, but not really any uglier than what we already do elsewhere when dealing with RelationType values, really. (E.g. we already have a static_cast<RelationType)(xpRelation) later in this function.)
Attachment #819405 - Flags: review?(surkov.alexander)
Attachment #819405 - Flags: review?(surkov.alexander) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
(Assignee)

Updated

5 years ago
Version: unspecified → Trunk
(Assignee)

Comment 3

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/0502bca5354d
Assignee: nobody → dholbert
Flags: in-testsuite-
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/0502bca5354d
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
(In reply to Daniel Holbert [:dholbert] from comment #1)
> It's absolutely correct that the compiler's erroring out on this, BTW - enum
> class types are supposed to be type-safe, so they can't be implicitly
> converted to int.

yeah, sorry I wasn't thinking when I reviewed that hunk :(

> (In reply to Daniel Holbert [:dholbert] from comment #0)
> > Presumably xpRelation's type should be changed.
> 
> Actually, it's not quite that simple; we initialize xpRelation as -1 (which
> is out of bounds for RelationType), so we can't change it unless we add a
> suitable "INVALID" value to its enum list that we could use in place of -1
> here.

actually another option that occurs to me know would be to make xpRelation's type MayBe<REelationType> but whatever someone can clean it up later
(Assignee)

Comment 6

5 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #5)
> actually another option that occurs to me know would be to make xpRelation's
> type MayBe<REelationType> but whatever someone can clean it up later

Ah, good point.

I actually feel gross enough about having added these static_casts that I'd like to immediately replace them with your Maybe<> suggestion. Followup coming up to do that.
(Assignee)

Comment 7

5 years ago
Created attachment 820017 [details] [diff] [review]
followup: use Maybe<> instead
Attachment #820017 - Flags: review?(trev.saunders)
(Assignee)

Comment 8

5 years ago
(pretend "followup" is in the commit message. I added it, but posted the patch before qreffing, and now my computer is being difficult so I'm not bothering to repost)
Comment on attachment 820017 [details] [diff] [review]
followup: use Maybe<> instead

I wish MayBe allowed you to just assign a T to it, but oh well
Attachment #820017 - Flags: review?(trev.saunders) → review+
You need to log in before you can comment on or make changes to this bug.