aaron.ballman added inline comments.
================ Comment at: lib/Sema/AnalysisBasedWarnings.cpp:1279 continue; - if (S.getLangOpts().CPlusPlus11) { + if (S.getLangOpts().CPlusPlus11 || S.getLangOpts().C99) { const Stmt *Term = B->getTerminatorStmt(); ---------------- xbolva00 wrote: > nickdesaulniers wrote: > > xbolva00 wrote: > > > nickdesaulniers wrote: > > > > 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=`? > > > Good point. > > IIUC, we allow GNU C attributes for C regardless of ISO vs GNU C. What we > > want to express is "if c++11 and newer, or c". > > > > I think this might be better expressed as: > > > > ``` > > if (S.getLangOpts().CPlusPlus11 || (!S.getLangOpts().CPlusPlus && > > !S.getLangOpts().ObjC) { > > ``` > > > > But maybe there's a more canonical way to express "if the language is C." > > (If not, maybe we can consider such a change to `LangOpt` to make this > > easier, but as part of this commit). > Yeah, I wanted to check if lang is C but I didnt find a nice way :( I think we want to support this attribute in ObjC because that language is based on C. In fact, I think we are supporting this attribute in all language modes regardless of -std level because the user can always use `__attribute__((fallthrough))` as a spelling. ================ Comment at: lib/Sema/AnalysisBasedWarnings.cpp:1233 + else + MacroName = "__attribute__ ((fallthrough))"; + } ---------------- I'd prefer to see this be `__attribute__((fallthrough))` without the extra whitespace. 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