Closed
Bug 987290
Opened 10 years ago
Closed 10 years ago
Allow using MFBT Typed Enums as bitwise flags
Categories
(Core :: MFBT, defect)
Core
MFBT
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: bjacob, Assigned: bjacob)
References
Details
Attachments
(4 files, 9 obsolete files)
38.44 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
2.34 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
889 bytes,
patch
|
Details | Diff | Splinter Review | |
2.77 KB,
patch
|
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Typing is good, it makes us catch real bugs. Watch the dependent bugs list here: this actually found a serious bug in gfx code. Traditionally, the way one allows using an enum as bitwise flags is by defining bitwise operators for it, as global functions. We already have a few occurences of that throughout gecko. The present patch adds a MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(EnumClass) macro that is all what one needs to use to enable using a given enum class as bit flags. It's not called by default i.e. usage as bitflags remains opt-in, needs to be manually enabled for each enum type. Notice the name, MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS, is intentionally explicit about what this does rather than what this is useful for, because I believe that it's good to have users aware of what macros actually do rather than rely on opaque magic. There is some nontrivialness here, that is explained in code comments, but let's quickly mention it here too. We have a lot of existing code doing if (myflags & SOME_FLAG) { ... Such code expects flags to be contextually convertible to bool. The problem is that there is no way to make typed enums convertible to bool: type conversion operators cannot be overloaded from the outside, and enum classes can't have members. The way we make this work, allowing existing user code to stay unchanged, is we have operator& and other bitwise binary operators have a return type that is a class that wraps the typed enum, and adds the bool conversion operator. That is BoolCastableTypedEnumResult. This depends on bug 987253 for 'explicit' specifiers on conversion operators. We could have made a different compromise, as explained in a code comment, that would have required adjusting user code. But since the goal here is to be an easy drop-in replacement for the untyped flags that we have all over the place, the value of allowing user code to stay unchanged seems quite high, IMHO high enough to justify having this admittedly crazy BoolCastableTypedEnumResult trick. That said, we are being careful here. To avoid this being a cause of inefficiency (*), we ensure that BoolCastableTypedEnumResult is a literal type in the C++11 sense (by sprinkling MOZ_CONSTEXPR) and we even verify that in the TestTypedEnum unit test, which is greatly expanded along the way. (*) : more about the possibility of inefficiency. Given how bit flags are used, I have little doubt that compilers will have no trouble optimizing BoolCastableTypedEnumResult away. I am a bit more concerned about the possibility of adding static constructors if people do things like this at file scope: const MyFlags kSomeSpecialFlagsCombo = MyFlags::A | MyFlags::B; Indeed, this constructs a BoolCastableTypedEnumResult at static initialization time. By ensuring that BoolCastableTypedEnumResult is a literal type, I hope to be doing the best I can to make the compiler understand that it can evaporate it away. And while literal types are a C++11 concept, this should help modern compilers regardless of being in c++11 mode. Suggestions welcome if something else needs to be done.
Attachment #8395826 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 1•10 years ago
|
||
Note that this patch also ensures that EnumClass::Enum is a literal type, which it wasn't before. For this reason, I believe that the reviewer should feel compelled to r+, otherwise the ultimate failure of Mozilla will be blamed on him.
Comment 2•10 years ago
|
||
Comment on attachment 8395826 [details] [diff] [review] Allow using typed enums as bit fields Review of attachment 8395826 [details] [diff] [review]: ----------------------------------------------------------------- ::: mfbt/TypedEnum.h @@ +322,5 @@ > > # define MOZ_BEGIN_ENUM_CLASS(...) MOZ_BEGIN_NESTED_ENUM_CLASS(__VA_ARGS__) > # define MOZ_END_ENUM_CLASS(Name) \ > MOZ_END_NESTED_ENUM_CLASS(Name) \ > + //MOZ_FINISH_NESTED_ENUM_CLASS(Name) ?
Assignee | ||
Comment 3•10 years ago
|
||
Excellent catch! I needed that at some point to get things to compile in the non-c++11-strong-enums case. I looked back at it; it turns out that the only problem is that the unary operator~ generated by this FINISH macro makes ~MyEnum::MyValue ambiguous with the new operator~ introduced here at least with clang 3.3. Apparently, being MOZ_DELETE'd doesn't remove an overload from the pool of overloads that might make an expression ambiguous. My quick fix is to remove this MOZ_DELETE'd operator~ from the FINISH macro. If you are not happy about this, then the alternative is to require using a different FINISH macro when the enum is meant to be used as bit field. But I liked the idea that enabling usage as bit field consists of nothing more than adding some operators to the existing typed enum.
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #8395826 -
Attachment is obsolete: true
Attachment #8395826 -
Flags: review?(jwalden+bmo)
Attachment #8395913 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 5•10 years ago
|
||
Needed to generate bitwise operators also for BoolCastableTypedEnumResult for chained ops like A & B & C to compile successfully on B2G GCC 4.4.
Attachment #8395913 -
Attachment is obsolete: true
Attachment #8395913 -
Flags: review?(jwalden+bmo)
Attachment #8396095 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8396095 [details] [diff] [review] Allow using typed enums as bit fields Temporarily cancelling this review while I'm fixing a bug. The operator bool in BoolCastableTypedEnumResult was kicking in more often than its turn, when the plain integer value of the enum was wanted, giving wrong results on more complex boolean expressions than the current unit test covered. Will come back with a fix (templated explicit conversion operator) and expanded unit test.
Attachment #8396095 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 7•10 years ago
|
||
Alright, the above-mentioned trick (have only one type conversion operator, explicit, templated in the destination type, to avoid causing any unwanted type coercion) works, and the unit test is now much expanded to cover all sorts of boolean expressions and all integer types. Ready for review!
Attachment #8396095 -
Attachment is obsolete: true
Attachment #8396422 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 8•10 years ago
|
||
Yet more fixes, needed to build on B2G GCC 4.4. Also, this version has the bitwise stuff added to a new TypedEnumBits.h file, so as to keep TypedEnum.h short and sweet (and cheap to include, although c++ templates *should* be cheap to include as long as we don't instantiate them).
Attachment #8396422 -
Attachment is obsolete: true
Attachment #8396422 -
Flags: review?(jwalden+bmo)
Assignee | ||
Updated•10 years ago
|
Attachment #8396702 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8396702 -
Attachment is obsolete: true
Attachment #8396702 -
Flags: review?(jwalden+bmo)
Attachment #8396705 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 10•10 years ago
|
||
Finally green on try! https://tbpl.mozilla.org/?tree=Try&rev=f96fcccad22d
Attachment #8396705 -
Attachment is obsolete: true
Attachment #8396705 -
Flags: review?(jwalden+bmo)
Attachment #8397029 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 11•10 years ago
|
||
So for a while I thought that it was horrible that my patch was making bitwise expressions on typed enums explicitly castable to other enum classes. Then I realize that in C++11, enum classes are explicitly castable to other enum classes. Here is still a new version of the patch: - adds unit test verifying that implicit conversions are not allowed (again, explicit conversions are intentionally allowed to follow c++11 enum class behavior) - adds unit test verifying that no truncation of high bits occurs - fixes a truncation bug in the non-c++11 impl of MOZ_BEGIN_ENUM_CLASS found by that unit test: the constructor taking an int was being implicitly used, coercing 64bit values to it. Fixed by templating that ctor on the integer type.
Assignee | ||
Comment 12•10 years ago
|
||
Attachment #8397029 -
Attachment is obsolete: true
Attachment #8397029 -
Flags: review?(jwalden+bmo)
Attachment #8397316 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 13•10 years ago
|
||
Attachment #8397316 -
Attachment is obsolete: true
Attachment #8397316 -
Flags: review?(jwalden+bmo)
Attachment #8398113 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 14•10 years ago
|
||
Review ping!
Comment 15•10 years ago
|
||
Comment on attachment 8398113 [details] [diff] [review] Allow using typed enums as bit fields Review of attachment 8398113 [details] [diff] [review]: ----------------------------------------------------------------- Generally pretty good, but we need to figure out the include story, and there's a bit of formatting-nicety that's needful. My eyes were glazing over by the end of TestTypedEnum.cpp, but it's probably good. :-) Except for the lack of handling of binary CTER<E> operators. Testing that would require some pairwise craziness. At least do some ad hoc testing of that; if you want to go whole hog and add exhaustive pairwise testing, feel free, but I'm not going to hold you to it. ::: mfbt/TypedEnumBits.h @@ +3,5 @@ > +/* This Source Code Form is subject to the terms of the Mozilla Public > + * License, v. 2.0. If a copy of the MPL was not distributed with this > + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > + > +/* MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS allows using a typed enum as bit flags */ Period at end of sentence. @@ +8,5 @@ > + > +#ifndef mozilla_TypedEnumBits_h > +#define mozilla_TypedEnumBits_h > + > +#include "mozilla/TypedEnum.h" Uh. These two headers circularly include each other. Which is the master, and which is the apprentice? If there's no clear answer, which there appears to not be, they need to be combined. @@ +9,5 @@ > +#ifndef mozilla_TypedEnumBits_h > +#define mozilla_TypedEnumBits_h > + > +#include "mozilla/TypedEnum.h" > +#include "mozilla/IntegerTypeTraits.h" Alphabetical. @@ +49,5 @@ > + : mValue(value) > + {} > + > + MOZ_CONSTEXPR operator E() const { return mValue; } > + MOZ_CONSTEXPR E get() const { return mValue; } Do we expect there will be actual calls to get()? Are there any now? I'd kind of rather there only be one way, that is, E(cter). @@ +60,5 @@ > + > + MOZ_CONSTEXPR bool operator !() const { return !bool(mValue); } > +}; > + > +#define BOOLCASTABLETYPEDENUMRESULT_BINARY_OPERATOR(Op, OtherType, ReturnType) \ MOZ_ prefix on this. Feel free to bikeshed a shorter name, too, if you can think of one. BINOP or something for concision. Don't care what. :-) @@ +62,5 @@ > +}; > + > +#define BOOLCASTABLETYPEDENUMRESULT_BINARY_OPERATOR(Op, OtherType, ReturnType) \ > +template <typename E> \ > +MOZ_CONSTEXPR ReturnType operator Op(const OtherType& e, const CastableTypedEnumResult<E>& r) \ Line break after ReturnType would be nice here, same elsewhere. @@ +64,5 @@ > +#define BOOLCASTABLETYPEDENUMRESULT_BINARY_OPERATOR(Op, OtherType, ReturnType) \ > +template <typename E> \ > +MOZ_CONSTEXPR ReturnType operator Op(const OtherType& e, const CastableTypedEnumResult<E>& r) \ > +{ \ > + return static_cast<ReturnType>(e Op static_cast<OtherType>(r)); \ OtherType(r) would be more readable, employed most places in this patch. @@ +86,5 @@ > +BOOLCASTABLETYPEDENUMRESULT_BINARY_OPERATOR(!=, E, bool) > +BOOLCASTABLETYPEDENUMRESULT_BINARY_OPERATOR(||, bool, bool) > +BOOLCASTABLETYPEDENUMRESULT_BINARY_OPERATOR(&&, bool, bool) > + > +#define BOOLCASTABLETYPEDENUMRESULT_COMPOUND_ASSIGN_OPERATOR(Op) \ Same MOZ_, shortening bit. @@ +88,5 @@ > +BOOLCASTABLETYPEDENUMRESULT_BINARY_OPERATOR(&&, bool, bool) > + > +#define BOOLCASTABLETYPEDENUMRESULT_COMPOUND_ASSIGN_OPERATOR(Op) \ > +template <typename E> \ > +void \ Technically |a &= b &= c;| normally works and groups as |a &= (b &= c);|, right? But we're not supporting it here. It's of dubious value, to be sure, just want to be sure we're wittingly doing this semi-deliberately as a simplification. I'm fine with it myself, chained assignments are often not super-readable, and not all languages implement it (e.g. JS has |a = b = c| as one or the other ordering of |<tmp> = c; a = <tmp>; b = <tmp>;|). @@ +105,5 @@ > +#undef BOOLCASTABLETYPEDENUMRESULT_BINARY_OPERATOR > + > +#ifndef MOZ_HAVE_CXX11_STRONG_ENUMS > + > +#define BOOLCASTABLETYPEDENUMRESULT_BINARY_OPERATOR_EXTRA_NON_CXX11(Op, ReturnType) \ MOZ_, shortening. Is there a reason we're not supporting (en & Enum::A) | (en & Enum::B) by not having an operator Op(const CTER<E>&, const CTER<E>&)? Or am I misreading/not thinking hard enough here? @@ +128,5 @@ > + > +#endif // not MOZ_HAVE_CXX11_STRONG_ENUMS > + > +namespace detail { > +template <typename E> template< ::: mfbt/tests/Makefile.in @@ +14,5 @@ > include $(topsrcdir)/config/rules.mk > > LIBS= $(call EXPAND_LIBNAME_PATH,mfbt,$(DEPTH)/mfbt) > + > +CXXFLAGS += -O0 -g3 \ No newline at end of file Why this at all? (And it may belong in moz.build these days as a per-file thing, if it belongs at all. Probably it doesn't.) ::: mfbt/tests/TestTypedEnum.cpp @@ +3,5 @@ > * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > > #include "mozilla/Assertions.h" > +#include "mozilla/TypedEnumBits.h" > + Add #include <stdint.h> up here. @@ +41,5 @@ > +RequireLiteralType(const T&) > +{ > + static_assert(std::is_literal_type<T>::value, "Expected a literal type"); > +} > +#else // not MOZ_HAVE_CXX11_STRONG_ENUMS That comment looks wrong. @@ +142,5 @@ > +MAKE_STANDARD_BITFIELD_FOR_TYPE(unsigned_long_long) > + > +#undef MAKE_STANDARD_BITFIELD_FOR_TYPE > + > +template <typename TypedEnum> template<
Attachment #8398113 -
Flags: review?(jwalden+bmo) → review-
Assignee | ||
Comment 16•10 years ago
|
||
Applied review comments; basically rewrote the unit test to avoid large macros and be much more systematic and exhaustive in its coverage.
Attachment #8398113 -
Attachment is obsolete: true
Attachment #8409731 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 17•10 years ago
|
||
mostly green try push: https://tbpl.mozilla.org/?tree=Try&rev=08e71f96996b The emulator-debug red is the subject of the next patch:
Assignee | ||
Comment 18•10 years ago
|
||
Ehsan: see the emulator-debug red on the above try link for why I need this. Regarding the question of "what changed, that made this necessary", the answer is "something in the big patch above", more specifically, in TypedEnum.h in the legacy non-c++11 implementation of MOZ_BEGIN/END_ENUM_CLASS. I tweaked various things that made typed enums more strict, and this audiochannel code was apparently relying on the ability to cast between unrelated enum types.
Attachment #8409734 -
Flags: review?(ehsan)
Assignee | ||
Comment 19•10 years ago
|
||
Right, I see it now. The old code in TypedEnum.h had explicit Name(int num) : mEnum((Enum)num) {} \ and the new code has: template<typename Other> \ explicit MOZ_CONSTEXPR Name(Other num) : mEnum((Enum)num) {} \ So the old code caused an implicit conversion to int, while the new code, being templated, does not implicitly convert to int, and instead tries to do an explicit conversion directly to the requested type. The old code was incorrect anyway as even plain c++98 enums can have a range exceeding that of int.
Updated•10 years ago
|
Attachment #8409734 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 20•10 years ago
|
||
Oops, comment 19 didn't make sense as the old code already had 'explicit'. Anyway, thanks for the r+ :-)
Assignee | ||
Comment 21•10 years ago
|
||
Fully green try run with both patches: https://tbpl.mozilla.org/?tree=Try&rev=87807f1f1ab5
Assignee | ||
Comment 22•10 years ago
|
||
This should be perfectly cromulent for review now.
Comment 23•10 years ago
|
||
Comment on attachment 8409731 [details] [diff] [review] Allow using typed enums as bit fields Review of attachment 8409731 [details] [diff] [review]: ----------------------------------------------------------------- ::: mfbt/TypedEnumBits.h @@ +15,5 @@ > +namespace mozilla { > + > +#define MOZ_CASTABLETYPEDENUMRESULT_BINOP(Op, OtherType, ReturnType) \ > +template<typename E> \ > +MOZ_CONSTEXPR ReturnType \ Nitpicky, but I'd rather not see the continuations aligned -- just makes more mess when they inevitably have to be realigned. @@ +29,5 @@ > +} \ > +template<typename E> \ > +MOZ_CONSTEXPR ReturnType \ > +operator Op(const CastableTypedEnumResult<E>& r1, \ > + const CastableTypedEnumResult<E>& r2) \ Misaligned. ::: mfbt/tests/TestTypedEnum.cpp @@ +4,5 @@ > > #include "mozilla/Assertions.h" > #include "mozilla/TypedEnum.h" > +#include "mozilla/TypedEnumBits.h" > +#include <stdint.h> Blank line before <stdint.h>. I sort of skimmed this file. :-) Looks generally reasonable enough, not hugely worried about some esoteric corner having been missed, it'll show up quickly enough in practice if so. @@ +303,5 @@ > +} > + > +// Similar to TestBinOp but testing the unary ~ operator. > +template<typename TypedEnum, typename T> > +void TestTilda(const T& t) Tilde
Attachment #8409731 -
Flags: review?(jwalden+bmo) → review+
Comment 24•10 years ago
|
||
(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #23) > ::: mfbt/TypedEnumBits.h > @@ +15,5 @@ > > +namespace mozilla { > > + > > +#define MOZ_CASTABLETYPEDENUMRESULT_BINOP(Op, OtherType, ReturnType) \ > > +template<typename E> \ > > +MOZ_CONSTEXPR ReturnType \ > > Nitpicky, but I'd rather not see the continuations aligned -- just makes > more mess when they inevitably have to be realigned. FWIW, emacs will smartly realign continuation backslashes when you indent the macro anyway. I assume vim can do the same.
Comment 25•10 years ago
|
||
<insert overwrought statement about editor snobbishness, and anointing particular editors as being things people should use, here -- and that unaligned backslashes are not inherently unreadable in any event :-) >
Assignee | ||
Comment 26•10 years ago
|
||
<insert statement about the joy of receiving r+ for this patch, and the ease of addressing this particular review comment by a regex>
Assignee | ||
Comment 27•10 years ago
|
||
Also, more generally: I only care about equivalence classes of code modulo applying invertible, automatable changes. You'll rarely see me argue strongly one way or the other between two options differing only in this way.
Assignee | ||
Comment 28•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/080ff1f86465 https://hg.mozilla.org/integration/mozilla-inbound/rev/4ad1f662f521
Assignee: nobody → bjacob
Comment 29•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/080ff1f86465 https://hg.mozilla.org/mozilla-central/rev/4ad1f662f521
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Comment 30•10 years ago
|
||
Backed out due to the same issues that bug 987311 had: https://hg.mozilla.org/mozilla-central/rev/c67a79064fd4 See bug 987311 comment 21, bug 987311 comment 23, bug 987311 comment 24.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 31•10 years ago
|
||
fyi, was seeing https://pastebin.mozilla.org/4975389
Updated•10 years ago
|
Target Milestone: mozilla31 → ---
Assignee | ||
Comment 32•10 years ago
|
||
(In reply to Blair McBride [:Unfocused] from comment #30) > Backed out due to the same issues that bug 987311 had: > https://hg.mozilla.org/mozilla-central/rev/c67a79064fd4 > > See bug 987311 comment 21, bug 987311 comment 23, bug 987311 comment 24. FWIW it doesn't seem that these, by themselves, imply that the present patches also had to be backed out, unless I'm missing something. Of course, it's only easy for me to tell because I've been working on these patches --- not complaining here. (In reply to Mark Capella [:capella] from comment #31) > fyi, was seeing https://pastebin.mozilla.org/4975389 Whoa, internal compiler error? That doesn't happen on our build slaves, or for me locally. Can you share details of your compiler version, linux distro, etc? maybe `c++ --version`. Also your mozconfig please.
Comment 33•10 years ago
|
||
Reproduced the error as I saw it in the Win side ... Win8U1 w/VS2012 Express for Win Desktop, produces: https://pastebin.mozilla.org/4982280
Assignee | ||
Comment 34•10 years ago
|
||
Regarding the internal compiler error on GCC 4.6: - I could not reproduce this on GCC 4.6.2 built from sources, with or without optimization. - This was only reported by someone on #introduction, not reproduced by anyone here AFAIU. So I'm not caring about this particular issue at this point. Looking into the MSVC issue reported in comment 33 now.
Assignee | ||
Comment 35•10 years ago
|
||
Hooray, I'm all set up on MSVC 2012 now and I can reproduce. And I have 30 days before my trial period expires. I could hardly be happier.
Assignee | ||
Comment 36•10 years ago
|
||
Alright, here's what was happening. These static asserts verify that CastableTypedEnumResult is not implicitly convertible to bool, int, etc. (It should only be explicitly convertible). However, that rests on the availability of c++11 explicit conversion operators. On bug 987253, I decided (from reading a stackoverflow thread, see the comment in the code) to stay away from exposing c++11 explicit conversions on MSVC, even on recent versions. The problem is that the TestTypedEnum.cpp test verifying non-implicit-convertibility forgot to check for availability of explicit conversions before requiring this behavior. The resulting patch is trivial enough that I don't think it requires a review.
Assignee | ||
Comment 37•10 years ago
|
||
Relanded: remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/38aa0e936d87 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/2b71adff7fe4 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/ba55dd0b1a12
Comment 38•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/38aa0e936d87 https://hg.mozilla.org/mozilla-central/rev/2b71adff7fe4 https://hg.mozilla.org/mozilla-central/rev/ba55dd0b1a12
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Updated•10 years ago
|
Target Milestone: mozilla32 → mozilla31
Assignee | ||
Comment 39•10 years ago
|
||
I was notified again of GCC 4.6 ICE's, now specifically on 4.6.3, so I built it myself and could reproduce. Landed a simple work-around: https://hg.mozilla.org/integration/mozilla-inbound/rev/ae2b85aaa9f3
Comment 40•10 years ago
|
||
Nice :-)
Could you land the workaround on aurora so people can build aurora with gcc 4.6?
Flags: needinfo?(bjacob)
Assignee | ||
Comment 42•10 years ago
|
||
[Approval Request Comment] Bug caused by (feature/regressing bug #): 987290, present bug User impact if declined: can't build on GCC 4.6.3 due to a GCC 4.6.3 bug. Other GCC versions such as 4.6.2 seem to be unaffected. Testing completed (on m-c, etc.): been on m-i for half a day. Risk to taking this patch (and alternatives if risky): Zero risk at all, completely trivial, and not part of the build we ship to users (only touching a unit test compiled as a separate executable) String or IDL/UUID changes made by this patch: none
Attachment #8414024 -
Flags: approval-mozilla-aurora?
Flags: needinfo?(bjacob)
Assignee | ||
Comment 43•10 years ago
|
||
Note: this landed on inbound as ae2b85aaa9f3.
Updated•10 years ago
|
status-firefox31:
--- → affected
status-firefox32:
--- → fixed
Updated•10 years ago
|
Attachment #8414024 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 45•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/ae2af33e459f
Assignee | ||
Comment 46•10 years ago
|
||
Yay, the 3 first hex digits of the aurora hash are identical to the central ones. It's going to be a lucky day.
You need to log in
before you can comment on or make changes to this bug.
Description
•