Closed
Bug 1329520
Opened 7 years ago
Closed 7 years ago
memory/mozalloc/throw_gcc.h:35:1: note: declaration missing '[[noreturn]]' (libc++ 4.0)
Categories
(Firefox Build System :: General, defect)
Tracking
(firefox-esr45 affected, thunderbird_esr45 affected, firefox50 wontfix, firefox51 wontfix, firefox52 fixed, firefox53 fixed, firefox54 fixed)
RESOLVED
FIXED
mozilla54
People
(Reporter: jbeich, Assigned: dimitry)
References
Details
Attachments
(2 files, 5 obsolete files)
749.16 KB,
text/plain
|
Details | |
3.92 KB,
patch
|
froydnj
:
review+
gchang
:
approval-mozilla-aurora+
jcristau
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
On platforms using config/gcc_hidden.h (per bug 273336) and with libc++ (FreeBSD and Android after bug 1260208) the build may break after https://github.com/llvm-mirror/libcxx/commit/14c09a2413ed $ cc -v FreeBSD clang version 4.0.0 (trunk 291274) (based on LLVM 4.0.0svn) Target: x86_64-unknown-freebsd12.0 Thread model: posix InstalledDir: /usr/bin Found CUDA installation: /usr/local/cuda, version 7.5 $ ./mach build [...] In file included from mozglue/misc/TimeStamp.cpp:11: In file included from objdir/dist/include/mozilla/TimeStamp.h:11: In file included from objdir/dist/stl_wrappers/algorithm:44: In file included from objdir/dist/system_wrappers/algorithm:3: In file included from /usr/include/c++/v1/algorithm:640: In file included from objdir/dist/stl_wrappers/memory:44: In file included from objdir/dist/system_wrappers/memory:3: In file included from /usr/include/c++/v1/memory:632: In file included from objdir/dist/stl_wrappers/new:44: In file included from objdir/dist/system_wrappers/new:3: /usr/include/c++/v1/new:132:1: error: function declared '[[noreturn]]' after its first declaration _LIBCPP_NORETURN _LIBCPP_FUNC_VIS void __throw_bad_alloc(); // not in C++ spec ^ /usr/include/c++/v1/__config:273:30: note: expanded from macro '_LIBCPP_NORETURN' # define _LIBCPP_NORETURN [[noreturn]] ^ objdir/dist/include/mozilla/throw_gcc.h:35:1: note: declaration missing '[[noreturn]]' attribute is here __throw_bad_alloc(void) ^ 1 error generated.
Comment hidden (mozreview-request) |
Comment on attachment 8824829 [details] Bug 1329520 - Switch MOZ_NORETURN to C++11-style attribute for compatibility with libc++ 4.0. https://reviewboard.mozilla.org/r/103096/#review103694 ::: js/public/Utility.h:41 (Diff revision 1) > namespace js {} > > #define JS_STATIC_ASSERT(cond) static_assert(cond, "JS_STATIC_ASSERT") > #define JS_STATIC_ASSERT_IF(cond, expr) MOZ_STATIC_ASSERT_IF(cond, expr, "JS_STATIC_ASSERT_IF") > > -extern MOZ_NORETURN MOZ_COLD JS_PUBLIC_API(void) > +MOZ_NORETURN MOZ_COLD extern JS_PUBLIC_API(void) Probably a result of namespace pollution. Applying to MFBT would aggravate such bustage. ```c++ In file included from ipc/contentproc/plugin-container.cpp:8: In file included from objdir/dist/include/nsXULAppAPI.h:15: In file included from objdir/dist/include/mozilla/XREAppData.h:12: In file included from objdir/dist/include/nsCOMPtr.h:33: In file included from objdir/dist/include/nsCycleCollectionNoteChild.h:13: In file included from objdir/dist/include/nsCycleCollectionTraversalCallback.h:11: In file included from objdir/dist/include/js/HeapAPI.h:15: objdir/dist/include/js/Utility.h:41:8: error: an attribute list cannot appear here extern MOZ_NORETURN MOZ_COLD JS_PUBLIC_API(void) ^~~~~~~~~~~~ objdir/dist/include/mozilla/throw_gcc.h:25:22: note: expanded from macro 'MOZ_NORETURN' #define MOZ_NORETURN _LIBCPP_NORETURN ^~~~~~~~~~~~~~~~ /usr/include/c++/v1/__config:273:28: note: expanded from macro '_LIBCPP_NORETURN' # define _LIBCPP_NORETURN [[noreturn]] ^~~~~~~~~~~~ 1 error generated. ```
Comment on attachment 8824829 [details] Bug 1329520 - Switch MOZ_NORETURN to C++11-style attribute for compatibility with libc++ 4.0. https://reviewboard.mozilla.org/r/103096/#review103694 Consider this v0. Actual fix is probably to use `[[noreturn]]` in MFBT.
Comment hidden (mozreview-request) |
The underlying issue here is a generic Clang vs. GCC incompatibility. However, libc++ is rarely used together with GCC and OS X opts out of using mozilla/throw_gcc.h by not defining WRAP_STL_INCLUDES. $ cat >a.cc #include <stdlib.h> __attribute__((noinline)) [[noreturn]] static void foo(void) { exit(0); } int main(void) { foo(); } $ g++6 -std=c++11 a.cc $ clang++ -std=c++11 a.cc a.cc:3:27: error: an attribute list cannot appear here __attribute__((noinline)) [[noreturn]] static void foo(void) ^~~~~~~~~~~~ 1 error generated.
Comment on attachment 8824829 [details] Bug 1329520 - Switch MOZ_NORETURN to C++11-style attribute for compatibility with libc++ 4.0. https://reviewboard.mozilla.org/r/103096/#review103694 ```c++ In file included from obj-firefox/gfx/tests/gtest/Unified_cpp_gfx_tests_gtest0.cpp:2: In file included from gfx/2d/unittest/TestBase.cpp:6: In file included from gfx/2d/unittest/TestBase.h:8: In file included from obj-firefox/dist/stl_wrappers/string:44: In file included from obj-firefox/dist/system_wrappers/string:3: In file included from clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.8.5/../../../../include/c++/4.8.5/string:40: In file included from clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.8.5/../../../../include/c++/4.8.5/bits/char_traits.h:39: In file included from clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.8.5/../../../../include/c++/4.8.5/bits/stl_algobase.h:64: In file included from clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.8.5/../../../../include/c++/4.8.5/bits/stl_pair.h:59: In file included from clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.8.5/../../../../include/c++/4.8.5/bits/move.h:57: In file included from obj-firefox/dist/stl_wrappers/type_traits:66: obj-firefox/dist/include/mozilla/throw_gcc.h:28:1: error: function declared '[[noreturn]]' after its first declaration MOZ_NORETURN MOZ_ALWAYS_INLINE MOZ_EXPORT void ^ obj-firefox/dist/include/mozilla/Attributes.h:112:33: note: expanded from macro 'MOZ_NORETURN' # define MOZ_NORETURN MOZ_HAVE_NORETURN ^ obj-firefox/dist/include/mozilla/Attributes.h:65:44: note: expanded from macro 'MOZ_HAVE_NORETURN' # define MOZ_HAVE_NORETURN [[noreturn]] ^ clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.8.5/../../../../include/c++/4.8.5/bits/functexcept.h:48:3: note: declaration missing '[[noreturn]]' attribute is here __throw_bad_exception(void) __attribute__((__noreturn__)); ^ ```
Attachment #8824829 -
Flags: review?(nfroyd)
Dimitry, is it possible to force libc++ use __attribute__((noreturn)) over [[return]] even in C++11 mode for compatibility with libstdc++? I can't test Clang on Linux behavior (outside of Try builds) because on FreeBSD --gcc-toolchain or -stdlib=libstdc++ are no longer supported.
Flags: needinfo?(dimitry)
status-firefox50:
--- → affected
status-firefox51:
--- → affected
status-firefox52:
--- → affected
status-firefox-esr45:
--- → affected
status-thunderbird_esr45:
--- → affected
Assignee | ||
Comment 8•7 years ago
|
||
(In reply to Jan Beich from comment #7) > Dimitry, is it possible to force libc++ use __attribute__((noreturn)) over > [[return]] even in C++11 mode for compatibility with libstdc++? The declaration in libc++ of its own macro for this purpose goes like this: #if __has_feature(cxx_attributes) # define _LIBCPP_NORETURN [[noreturn]] #else # define _LIBCPP_NORETURN __attribute__ ((noreturn)) #endif and the cxx_attributes feature is always enabled for C++11. That said, the complaint from comment 6 seems to indicate that it doesn't accept "mixing and matching" __attribute__((noreturn)) and [[noreturn]] either. For example, this little fragment: __attribute__((noreturn)) void foo(void); [[noreturn]] void foo(void); isn't accepted by clang in C++11 mode: foo.cpp:2:3: error: function declared '[[noreturn]]' after its first declaration [[noreturn]] void foo(void); ^ foo.cpp:1:32: note: declaration missing '[[noreturn]]' attribute is here __attribute__((noreturn)) void foo(void); ^ 1 error generated. while g++ 6.2.0 does accept it. I'm unsure what to do here. One question is why does throw_gcc.h try to redeclare __throw_bad_exception at all? Can't it just use the declaration from the system headers?
Comment 9•7 years ago
|
||
From the header itself: // For gcc, we define these inline to abort so that we're absolutely // certain that (i) no exceptions are thrown from Gecko; (ii) these // errors are always terminal and caught by breakpad.
Reporter | ||
Comment 10•7 years ago
|
||
(In reply to Dimitry Andric from comment #8) > isn't accepted by clang in C++11 mode: > > foo.cpp:2:3: error: function declared '[[noreturn]]' after its first > declaration > [[noreturn]] void foo(void); > ^ > foo.cpp:1:32: note: declaration missing '[[noreturn]]' attribute is here > __attribute__((noreturn)) void foo(void); __attribute__((foo)) often translates to [[gnu::foo]] but [[noreturn]] is special because it's also part of C++11. If [[gnu::noreturn]] and [[noreturn]] have different semantics then it's understandable why Clang throws an error. However, the following is accepted [[noreturn]] void foo(void); [[gnu::noreturn]] void foo(void); but if swapped it's not [[gnu::noreturn]] void foo(void); [[noreturn]] void foo(void);
Attachment #8824829 -
Attachment is obsolete: true
Assignee | ||
Comment 11•7 years ago
|
||
I've proposed a fix for libc++ here: https://reviews.llvm.org/D28981
Comment 12•7 years ago
|
||
__throw_bad_alloc is an internal implementation detail of libstdc++ and mozilla has no business whatsoever trying to redefine it. Get off my lawn.
Comment 13•7 years ago
|
||
But if you do insist on this kind of foul play, it's your job to get it right, you can't expect standard library implementations to adjust their implementation details to accommodate misuse of them. I'm half tempted to randomly add and remove attributes from the libstdc++ header to prevent this kind of tomfoolery.
Comment 14•7 years ago
|
||
I suggest making a feature request to libstdc++ so that (for example) the functions get defined as inline when a particular macro is defined. That would be a supported API for doing what you want, not a hack that subverts the standard library's implementation details. I'm certainly in favour of making -fno-exceptions more useful with libstdc++ (because currently you need to recompile libstdc++ itself to really get no exceptions). But not by some unsupported backdoor.
Comment 15•7 years ago
|
||
(In reply to Jonathan Wakely from comment #14) > I suggest making a feature request to libstdc++ so that (for example) the > functions get defined as inline when a particular macro is defined. That > would be a supported API for doing what you want, not a hack that subverts > the standard library's implementation details. This probably would have been better than whatever we do now. But sometimes the expedient hack is more useful than the proper fix (especially with the GCC review -> release -> usable in Firefox lead cycle). Propriety of futzing with implementation details aside, what we have at least doesn't change the contract of the implementation-defined functions, but simply makes them usable with -fno-exceptions. There's a bug filed for this upstream: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69388 > I'm certainly in favour of making -fno-exceptions more useful with libstdc++ > (because currently you need to recompile libstdc++ itself to really get no > exceptions). But not by some unsupported backdoor. We would love to work on a solution that's acceptable to the libstdc++ maintainers. But this particular bug is probably not the place to do that.
Comment 16•7 years ago
|
||
(In reply to Nathan Froyd [:froydnj] from comment #15) > There's a bug filed for this upstream: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69388 Oh yes, I created that last time I was offended by these replacements. > > I'm certainly in favour of making -fno-exceptions more useful with libstdc++ > > (because currently you need to recompile libstdc++ itself to really get no > > exceptions). But not by some unsupported backdoor. > > We would love to work on a solution that's acceptable to the libstdc++ > maintainers. But this particular bug is probably not the place to do that. Agreed, the GCC bug is where it belongs. Thanks for reminding me of it.
Assignee | ||
Comment 17•7 years ago
|
||
Here's the patch I have submitted to the FreeBSD bug. This prefixes all the __throw_xxx functions with [[noreturn]], but *only* for clang, and specifically for libc++ 4.0.0 and higher.
Reporter | ||
Comment 18•7 years ago
|
||
Comment on attachment 8829928 [details] [diff] [review] mozcentral-fix-libc++-noreturn-1.diff How hard it'd be to fix comment 10 instead? Does MSVC accept the following form unlike clang-cl? void foo(void); [[noreturn]] void foo(void); Otherwise, mozilla/throw_gcc.h can be turned off for more libc++ platforms following OS X due to lacking wrappers for __throw_bad_any_cast __throw_bad_array_length __throw_bad_optional_access __throw_bad_variant_access __throw_bad_weak_ptr
Comment 19•7 years ago
|
||
(In reply to Nathan Froyd [:froydnj] from comment #15) > This probably would have been better than whatever we do now. But sometimes > the expedient hack is more useful than the proper fix (especially with the > GCC review -> release -> usable in Firefox lead cycle). Considering we rely on system libstdc++, the "usable in Firefox lead cycle" starts with "is available on all supported distros".
Reporter | ||
Comment 20•7 years ago
|
||
Comment on attachment 8829928 [details] [diff] [review] mozcentral-fix-libc++-noreturn-1.diff this patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=680ee93c3962d1514583d7f4534e435f495fd00a alternative: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8c6062bbaa6e5a3c0f13af152f387ba30ba2375c
Comment 21•7 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #19) > (In reply to Nathan Froyd [:froydnj] from comment #15) > > This probably would have been better than whatever we do now. But sometimes > > the expedient hack is more useful than the proper fix (especially with the > > GCC review -> release -> usable in Firefox lead cycle). > > Considering we rely on system libstdc++, the "usable in Firefox lead cycle" > starts with "is available on all supported distros". That doesn't make it right to rely on the hacks forever. Start the conversation upstream and eventually it will be fixed and you can remove these hacks. Don't have the conversation and you'll be trying to be compatible with internal implementation details (on all supported distros!) forever.
Assignee | ||
Comment 22•7 years ago
|
||
The patch I submitted in comment 17 did not compile with gcc, since it does not recognize __has_feature(), sorry about that. Here is a second patch, that ensures __has_feature is defined if it isn't available.
Attachment #8829928 -
Attachment is obsolete: true
Comment 23•7 years ago
|
||
It's undefined behaviour to #define __has_feature, and even ignoring that it's a bad idea to do so in a header because other code might try to use it and could assume the fact it's defined means it's supported by the compiler. Mozilla headers should not be defining macros in the implementation namespace. Mozilla is not the C++ implementation. This seems a better approach: #if defined(__clang__) # if __has_feature(cxx_attributes) \ && defined(_LIBCPP_VERSION) && _LIBCPP_VERSION >= 4000 # define MOZ_NORETURN_CXX11 [[noreturn]] # endif #endif #ifndef MOZ_NORETURN_CXX11 # define MOZ_NORETURN_CXX11 #endif
Assignee | ||
Comment 24•7 years ago
|
||
I'm fine with any approach that works, so that looks good to me. (It could be that very old versions of clang don't support __has_feature(), but we should probably not bother with those anyway.)
Assignee | ||
Comment 25•7 years ago
|
||
Updated with Jonathan's suggestion.
Attachment #8830273 -
Attachment is obsolete: true
Reporter | ||
Comment 26•7 years ago
|
||
Comment on attachment 8830499 [details] [diff] [review] Use [[noreturn]] for __throw_xxx wrappers, if libc++ 4.0.0 or higher https://treeherder.mozilla.org/#/jobs?repo=try&revision=702a46a802a1920fd46bf40158b0744e232b0d07
Attachment #8830499 -
Flags: review?(nfroyd)
Comment 27•7 years ago
|
||
Comment on attachment 8830499 [details] [diff] [review] Use [[noreturn]] for __throw_xxx wrappers, if libc++ 4.0.0 or higher Review of attachment 8830499 [details] [diff] [review]: ----------------------------------------------------------------- This seems mostly OK. I'm cancelling the review just because I want to know whether my suggestion below will work, but if it's not feasible, I have no problem r+'ing the revised patch. ::: memory/mozalloc/throw_gcc.h @@ +25,5 @@ > +# define MOZ_NORETURN_CXX11 [[noreturn]] > +# endif > +#endif > +#ifndef MOZ_NORETURN_CXX11 > +# define MOZ_NORETURN_CXX11 Whatever macro we #define here, we need to #undef at the end of the file. Also, there needs to be a comment somewhere about why we have this separate noreturn attribute for these cases. Can we do something like: #if defined(__clang__) ... #define MOZ_NORETURN_THROW [[noreturn]] #endif #ifndef MOZ_NORETURN_THROW # define MOZ_NORETURN_THROW MOZ_NORETURN #endif MOZ_EXPORT MOZ_NORETURN_THROW MOZ_ALWAYS_INLINE void __throw_foo(void) { ... } ? We might have to swap the order of some of these depending on how [[attribute]] in the grammer intermixes with __attribute__, but that seems nicer to read than MOZ_NORETURN_CXX11...MOZ_NORETURN.
Attachment #8830499 -
Flags: review?(nfroyd) → feedback+
Assignee | ||
Comment 28•7 years ago
|
||
(In reply to Nathan Froyd [:froydnj] from comment #27) > ::: memory/mozalloc/throw_gcc.h > @@ +25,5 @@ > > +# define MOZ_NORETURN_CXX11 [[noreturn]] > > +# endif > > +#endif > > +#ifndef MOZ_NORETURN_CXX11 > > +# define MOZ_NORETURN_CXX11 > > Whatever macro we #define here, we need to #undef at the end of the file. Ah yes, understood. > Also, there needs to be a comment somewhere about why we have this separate > noreturn attribute for these cases. Sure, I can add that just before the #ifdef block. > Can we do something like: > > #if defined(__clang__) > ... #define MOZ_NORETURN_THROW [[noreturn]] > #endif > #ifndef MOZ_NORETURN_THROW > # define MOZ_NORETURN_THROW MOZ_NORETURN > #endif > > MOZ_EXPORT MOZ_NORETURN_THROW MOZ_ALWAYS_INLINE void > __throw_foo(void) > { > ... > } > > ? We might have to swap the order of some of these depending on how > [[attribute]] in the grammer intermixes with __attribute__, but that seems > nicer to read than MOZ_NORETURN_CXX11...MOZ_NORETURN. Well, the [[noreturn]] attribute must appear first on the line, before any __attribute__ ones, otherwise clang won't accept it. (Apparently gcc has no such qualms. :) I tried to avoid swapping the MOZ_EXPORT and MOZ_NORETURN, since I am not entirely sure how that plays out on e.g. Windows, where MOZ_EXPORT expands to __declspec(export), I believe. Does Microsoft C++ accept __declspec(noreturn) before __declspec(export)? If so, your approach is probably fine.
Comment 29•7 years ago
|
||
(In reply to Dimitry Andric from comment #28) > (In reply to Nathan Froyd [:froydnj] from comment #27) > > Can we do something like: > > > > #if defined(__clang__) > > ... #define MOZ_NORETURN_THROW [[noreturn]] > > #endif > > #ifndef MOZ_NORETURN_THROW > > # define MOZ_NORETURN_THROW MOZ_NORETURN > > #endif > > > > MOZ_EXPORT MOZ_NORETURN_THROW MOZ_ALWAYS_INLINE void > > __throw_foo(void) > > { > > ... > > } > > > > ? We might have to swap the order of some of these depending on how > > [[attribute]] in the grammer intermixes with __attribute__, but that seems > > nicer to read than MOZ_NORETURN_CXX11...MOZ_NORETURN. > > Well, the [[noreturn]] attribute must appear first on the line, before any > __attribute__ ones, otherwise clang won't accept it. (Apparently gcc has no > such qualms. :) That's unfortunate, but I guess it makes some things inside clang easier. > I tried to avoid swapping the MOZ_EXPORT and MOZ_NORETURN, since I am not > entirely sure how that plays out on e.g. Windows, where MOZ_EXPORT expands > to __declspec(export), I believe. Does Microsoft C++ accept > __declspec(noreturn) before __declspec(export)? If so, your approach is > probably fine. I don't know. I suspect that it does, since they're both just __declspec and for there to be an ordering requirement between __declspecs would be...unusual. I am willing to try test patches for you, or you can request tryserver access: https://wiki.mozilla.org/ReleaseEngineering/TryServer and push test patches yourself.
Assignee | ||
Comment 30•7 years ago
|
||
Updated with Nathan's suggestions from comment 27: * Add explanatory comment before defining MOZ_NORETURN_THROW * Use only one instance of MOZ_NORETURN_THROW, at the start of function declarations * Undefine MOZ_NORETURN_THROW at the end of the header
Attachment #8830499 -
Attachment is obsolete: true
Reporter | ||
Comment 31•7 years ago
|
||
Comment on attachment 8831204 [details] [diff] [review] Use [[noreturn]] for __throw_xxx wrappers, if libc++ 4.0.0 or higher https://treeherder.mozilla.org/#/jobs?repo=try&revision=e5e46bbf599a92fb32e34baa12a9882dcd8c5043
Attachment #8831204 -
Flags: review?(nfroyd)
Comment 32•7 years ago
|
||
Comment on attachment 8831204 [details] [diff] [review] Use [[noreturn]] for __throw_xxx wrappers, if libc++ 4.0.0 or higher Review of attachment 8831204 [details] [diff] [review]: ----------------------------------------------------------------- This looks fine; two issues noted below. ::: memory/mozalloc/throw_gcc.h @@ +37,5 @@ > // NB: user code is not supposed to touch the std:: namespace. We're > // doing this after careful review because we want to define our own > // exception throwing semantics. Don't try this at home! > > +MOZ_NORETURN_THROW MOZ_EXPORT MOZ_ALWAYS_INLINE void I think this reads slightly better as MOZ_THROW_NORETURN; the MOZ_NORETURN at the beginning of the line threw me, and then I realized there was an underscore following it, instead of the space. WDYT? Can you s/MOZ_NORETURN_THROW/MOZ_THROW_NORETURN/g if you agree? @@ +140,4 @@ > > } // namespace std > > +#undef MOZ_RETURN_THROW Surely this should be MOZ_NORETURN_THROW?
Attachment #8831204 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 33•7 years ago
|
||
(In reply to Nathan Froyd [:froydnj] from comment #32) > > +MOZ_NORETURN_THROW MOZ_EXPORT MOZ_ALWAYS_INLINE void > > I think this reads slightly better as MOZ_THROW_NORETURN; the MOZ_NORETURN > at the beginning of the line threw me, and then I realized there was an > underscore following it, instead of the space. > > WDYT? Can you s/MOZ_NORETURN_THROW/MOZ_THROW_NORETURN/g if you agree? Sure, substituted. > > +#undef MOZ_RETURN_THROW > > Surely this should be MOZ_NORETURN_THROW? Yes, that was a copy/paste error. Fixed it now.
Attachment #8831204 -
Attachment is obsolete: true
Reporter | ||
Comment 34•7 years ago
|
||
Comment on attachment 8831309 [details] [diff] [review] Use [[noreturn]] for __throw_xxx wrappers, if libc++ 4.0.0 or higher https://treeherder.mozilla.org/#/jobs?repo=try&revision=1504807bebe2f7cc0e49ee39ac829f0452acf6e2
Attachment #8831309 -
Flags: review?(nfroyd)
Updated•7 years ago
|
Attachment #8831309 -
Flags: review?(nfroyd) → review+
Comment 35•7 years ago
|
||
Thank you!
Keywords: checkin-needed
Comment 36•7 years ago
|
||
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/3b96baf833a5 Fix "memory/mozalloc/throw_gcc.h:35:1: note: declaration missing '[[noreturn]]' (libc++ 4.0)" r=nfroyd
Keywords: checkin-needed
Comment 37•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3b96baf833a5
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Reporter | ||
Comment 38•7 years ago
|
||
Comment on attachment 8831309 [details] [diff] [review] Use [[noreturn]] for __throw_xxx wrappers, if libc++ 4.0.0 or higher Would be nice to have on ESR52 to help regression testing downstream. For ESR45 it's too late (only 2 releases are left) and the patch doesn't apply cleanly, anyway. Approval Request Comment [Feature/Bug causing the regression]: None [User impact if declined]: Broken build on platforms shipping Clang/libc++ 4.0 by default e.g., FreeBSD 11.1, OS X 10.13 [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: Yes, by me and dowstream e.g., http://package18.nyi.freebsd.org/data/headi386PR216008-default/2017-02-01_06h23m11s/logs/firefox-51.0.1,1.log [Needs manual test from QE? If yes, steps to reproduce]: Probably not. [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: Can only break build for Linux and Android. Windows uses memory/mozalloc/throw_msvc.h instead while OS X - neither. [String changes made/needed]: None
Attachment #8831309 -
Flags: approval-mozilla-beta?
Attachment #8831309 -
Flags: approval-mozilla-aurora?
Comment 39•7 years ago
|
||
Comment on attachment 8831309 [details] [diff] [review] Use [[noreturn]] for __throw_xxx wrappers, if libc++ 4.0.0 or higher Fix a build issue on platforms shipping Clang/libc++ 4.0 by default. Aurora53+.
Attachment #8831309 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 40•7 years ago
|
||
Comment on attachment 8831309 [details] [diff] [review] Use [[noreturn]] for __throw_xxx wrappers, if libc++ 4.0.0 or higher work around libc++ wonkiness, beta52+
Attachment #8831309 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 41•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/c9c4eb9fbaeb
Comment 42•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/87b018eade46
Comment 43•7 years ago
|
||
Too late for 51. Mark 51 won't fix.
Updated•6 years ago
|
Product: Core → Firefox Build System
Assignee | ||
Updated•11 months ago
|
Flags: needinfo?(dimitry)
You need to log in
before you can comment on or make changes to this bug.
Description
•