[PATCH] D63260: [Attr] Support _attribute__ ((fallthrough))

[Aaron Ballman via Phabricator via cfe-commits]

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