Message ID | 6619e7e2-248e-08bc-6dc5-7b4d11bea3df@auburn.edu |
---|---|
State | New |
Headers | show |
On 03/19/2017 10:44 PM, Justin Brewer wrote: > -# if !defined __GNUC__ || defined __STRICT_ANSI__ > +# if !(defined __GNUC__ && __GNUC_PREREQ(4,8)) || defined __STRICT_ANSI__ I'm not happy how this disables the useful = warning for clang altogether. Do we have a better option? Is there a way for check for the -pedantic flag? Thanks, Florian
On Tue, 21 Mar 2017, Florian Weimer wrote: > On 03/19/2017 10:44 PM, Justin Brewer wrote: > > -# if !defined __GNUC__ || defined __STRICT_ANSI__ > > +# if !(defined __GNUC__ && __GNUC_PREREQ(4,8)) || defined __STRICT_ANSI__ > > I'm not happy how this disables the useful = warning for clang altogether. > > Do we have a better option? Is there a way for check for the -pedantic flag? By design, warning options such as -pedantic are not meant to affect code generation, so should not have any way to check for them within the code. (Preferably __extension__ should be used, but that runs into the issues with disabling it for the user's expression as discussed earlier for this code <https://sourceware.org/ml/libc-alpha/2016-11/msg00895.html>.)
On 03/21/2017 05:21 PM, Joseph Myers wrote: > On Tue, 21 Mar 2017, Florian Weimer wrote: > >> On 03/19/2017 10:44 PM, Justin Brewer wrote: >>> -# if !defined __GNUC__ || defined __STRICT_ANSI__ >>> +# if !(defined __GNUC__ && __GNUC_PREREQ(4,8)) || defined __STRICT_ANSI__ >> >> I'm not happy how this disables the useful = warning for clang altogether. >> >> Do we have a better option? Is there a way for check for the -pedantic flag? > > By design, warning options such as -pedantic are not meant to affect code > generation, so should not have any way to check for them within the code. > > (Preferably __extension__ should be used, but that runs into the issues > with disabling it for the user's expression as discussed earlier for this > code <https://sourceware.org/ml/libc-alpha/2016-11/msg00895.html>.) I think we can use __extension__ if we also expand the expression without __extension__ in an unevaluated context. The tricky part is to find one that is independent of GNU extensions. Perhaps this would work? # define assert(expr) \ ((void) sizeof ((expr) == 0), __extension__ ({ \ if (expr) \ ; /* empty */ \ else \ __assert_fail (#expr, __FILE__, __LINE__, __FUNCTION__); \ })) sizeof suppresses the evaluation of the first occurrence of expr. The comparison is needed because sizeof cannot be applied to function pointers and bitfields. C11 says that expr is compared to zero, so the (expr) == 0 expression is well-formed. What do you think? Should we make this change? Thanks, Florian
On Sun, 25 Jun 2017, Florian Weimer wrote: > I think we can use __extension__ if we also expand the expression > without __extension__ in an unevaluated context. The tricky part is to > find one that is independent of GNU extensions. > Perhaps this would work? > > # define assert(expr) \ > ((void) sizeof ((expr) == 0), __extension__ ({ \ > if (expr) \ > ; /* empty */ \ > else \ > __assert_fail (#expr, __FILE__, __LINE__, __FUNCTION__); \ > })) > > sizeof suppresses the evaluation of the first occurrence of expr. The > comparison is needed because sizeof cannot be applied to function > pointers and bitfields. C11 says that expr is compared to zero, so the > (expr) == 0 expression is well-formed. > > What do you think? Should we make this change? I think that's reasonable (appropriately commented to explain why it's done that way).
On 06/26/2017 12:56 PM, Joseph Myers wrote: > On Sun, 25 Jun 2017, Florian Weimer wrote: > >> I think we can use __extension__ if we also expand the expression >> without __extension__ in an unevaluated context. The tricky part is to >> find one that is independent of GNU extensions. >> Perhaps this would work? >> >> # define assert(expr) \ >> ((void) sizeof ((expr) == 0), __extension__ ({ \ >> if (expr) \ >> ; /* empty */ \ >> else \ >> __assert_fail (#expr, __FILE__, __LINE__, __FUNCTION__); \ >> })) >> >> sizeof suppresses the evaluation of the first occurrence of expr. The >> comparison is needed because sizeof cannot be applied to function >> pointers and bitfields. C11 says that expr is compared to zero, so the >> (expr) == 0 expression is well-formed. >> >> What do you think? Should we make this change? > > I think that's reasonable (appropriately commented to explain why it's > done that way). This is what I came up with as a patch. I would consider this still appropriate during the freeze. It's something we'd backport to glibc 2.25, too. Thanks, Florian
diff --git a/assert/assert.h b/assert/assert.h index 22f019537c..88dde76414 100644 --- a/assert/assert.h +++ b/assert/assert.h @@ -85,7 +85,7 @@ __END_DECLS /* When possible, define assert so that it does not add extra parentheses around EXPR. Otherwise, those added parentheses would suppress warnings we'd expect to be detected by gcc's -Wparentheses. */ -# if !defined __GNUC__ || defined __STRICT_ANSI__ +# if !(defined __GNUC__ && __GNUC_PREREQ(4,8)) || defined __STRICT_ANSI__ # define assert(expr) \ ((expr) \ ? __ASSERT_VOID_CAST (0) \