This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On Wed, Jul 16, 2014 at 1:43 PM, Jim Meyering <jim@meyering.net> wrote: > On Wed, Jul 16, 2014 at 1:15 PM, Roland McGrath <roland@hack.frob.com> wrote: >> This certainly should not go in during the current release freeze. >> So you might not want to try to get too much discussion going until >> the floodgates reopen. Also, various key people might be less than >> usually responsive until after Cauldron (and for me, another week of >> vacation thereafter). > > Thanks for the quick feedback. > >> Isn't there an "empty statement" warning that might be on too? We >> certainly don't want the internals of the macro (the then clause in >> your new version) to produce warnings of their own with any possible >> combination of switches. > > -Wempty-body does not complain about the empty "then" clause > inside that statement-expression. > >> You should be able to use __extension__ for GCC under -ansi. But >> perhaps that would wrongly allow use of extensions inside the user's >> expression too. If you're avoiding it for good reason, there should >> be comments there explaining why. > > Good point. I confirmed that prefixing the ({... with __extension__ > lets me eliminate the __STRICT_ANSI__ disjunct, and that -ansi > still warns about a use like "assert( ({1}) );". I'll update the patch > and repost here -- with a detailed test description -- > in a couple weeks. > >> I also wonder how modern GCCs running in integrated preprocessor >> mode deal with the system header exception and/or __extension__ for >> this sort of case (since in integrated preprocessor mode it can tell >> which part came from the system header and which from the user). >> >> Any change like this is going to need a lot of detailed reporting >> about testing with different compilers and option combinations and >> sorts of expressions (i.e. ones using extensions or not and perhaps >> various different sorts of extensions). > > I've tested it with -Wextra -Wall -W -O, with uses as statements and > as expressions. Also with and without -ansi. I have not yet tested > with a non-__GNUC__ compiler, but will do. Hi Roland, This slipped off my radar (for two years!). I've attached an updated patch. We *do* need that __STRICT_ANSI__ disjunct. Otherwise, this would evoke no warning: $ gcc -isystem. -I. -Werror=pedantic k.c In file included from k.c:1:0: k.c: In function ‘main’: k.c:2:23: warning: ISO C forbids braced-groups within expressions [-Wpedantic] int main() { assert ( ({1;}) ); return 0; } Tests I ran manually in a directory with the new assert.h file: Ensure that "assert(a=0)" now elicits a warning and this fails: printf '%s\n' '#include <assert.h>' \ 'int main() {int a = 1; assert (a = 0); return a;}' > k.c gcc -c -I. -Werror=parentheses k.c Ensure that clang does the same: clang -c -I. -Werror=parentheses k.c Ensure that this fails with a diagnostic about the stmt expression: printf '%s\n' '#include <assert.h>' \ 'int main() { assert (({1;})); return 0; }' > k.c gcc -c -isystem. -I. -Werror=pedantic k.c Ensure -Werror=pedantic does not complain about the stmt expression: printf '%s\n' '#include <assert.h>' \ 'int main() { assert (0); return 0; }' > k.c gcc -isystem. -I. -Werror=pedantic k.c I didn't see any other tests for failing compilation. Do you require a test suite addition for these? If so, would a single bourne shell script be acceptable?
Attachment:
glibc-assert--do-not-suppress-warning-about-accidental-assignment.diff
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |