nickdesaulniers added a comment. Thanks for the patch, I look forward to this feature!
I think the changes in `test/SemaCXX/warn-unused-label-error.cpp`, `test/Sema/block-literal.c`, and `test/Sema/address_spaces.c` should not be committed (2 look like unrelated cleanups?). ================ Comment at: lib/Parse/ParseStmt.cpp:110-111 assert((Attrs.empty() || Res.isInvalid() || Res.isUsable()) && "attributes on empty statement"); ---------------- Wouldn't this statement have to change? `__attribute__((fallthrough))` //is// an "attribute on [an] empty statement." Maybe this is not the correct place to try to parse a GNU attr? ================ Comment at: lib/Sema/AnalysisBasedWarnings.cpp:1279 continue; - if (S.getLangOpts().CPlusPlus11) { + if (S.getLangOpts().CPlusPlus11 || S.getLangOpts().C99) { const Stmt *Term = B->getTerminatorStmt(); ---------------- Probably should additionally/instead check `S.getLangOpts().GNUMode` (since these are GNU C style attributes)? I guess we want these attributes to be supported in C regardless of `-std=`? ================ Comment at: test/Sema/block-literal.c:44 takeblock(^{ x = 4; }); // expected-error {{variable is not assignable (missing __block type specifier)}} - __block y = 7; // expected-warning {{type specifier missing, defaults to 'int'}} - takeblock(^{ y = 8; }); + __block y = 7; // expected-error {{use of undeclared identifier 'y'}} + takeblock(^{ y = 8; }); // expected-error {{use of undeclared identifier 'y'}} ---------------- xbolva00 wrote: > I tried look at this, but I have no idea how my change to parse GNU affects > __block. If you remove your change to `lib/Parse/ParseStmt.cpp`, does this test still fail in this way? Did you test this on an assertion enabled build (`-DLLVM_ENABLE_ASSERTIONS=ON`)? (I would have expected the assertion I commented on above to fail.) ================ Comment at: test/Sema/fallthrough-attr.c:1 +// RUN: %clang_cc1 -fsyntax-only -std=c99 -verify -Wimplicit-fallthrough %s +// RUN: %clang_cc1 -fsyntax-only -std=c11 -verify -Wimplicit-fallthrough %s ---------------- would you mind adding a run for `-std=gnu89`? In particular, I'm worried about you're above change checking c99. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63260/new/ https://reviews.llvm.org/D63260 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits