On Tue, Mar 31, 2015 at 5:55 PM, Nico Weber <[email protected]> wrote: > Hi aaron.ballman, > > Fixes PR23089
Thank you for working on this! > > http://reviews.llvm.org/D8750 > > Files: > include/clang/Basic/Attr.td > include/clang/Basic/DiagnosticSemaKinds.td > include/clang/Sema/AttributeList.h > test/SemaCXX/switch-implicit-fallthrough.cpp > > Index: include/clang/Basic/Attr.td > =================================================================== > --- include/clang/Basic/Attr.td > +++ include/clang/Basic/Attr.td > @@ -106,6 +106,8 @@ > def GlobalVar : SubsetSubject<Var, > [{S->hasGlobalStorage()}]>; > > +def NotAllowedOnDecls : SubsetSubject<Var, [{false}]>; > + > // FIXME: this hack is needed because DeclNodes.td defines the base Decl node > // type to be a class, not a definition. This makes it impossible to create > an > // attribute subject which accepts a Decl. Normally, this is not a problem, > @@ -690,7 +692,8 @@ > > def FallThrough : Attr { > let Spellings = [CXX11<"clang", "fallthrough">]; > -// let Subjects = [NullStmt]; > + let Subjects = SubjectList<[NotAllowedOnDecls], ErrorDiag, > + "ExpectedEmptyStatement">; This does not seem like the correct fix to me. For starters, not on decls != allowed on statements (for instance, this suggests it could be applied to types as well). Also, while I know we don't have a lot of automation in SemaStmtAttr.cpp for this, we want to keep the declarative nature of the td file accurate, and fix the source code instead (as NullStmt is the only valid Stmt this attribute can attach to, but the declaration in the td file appears like you could put this on anything that's not a declaration). I'm traveling for work this week, and don't have the chance to look into the code too heavily, but I suspect this is a fix that would go into handleCommonAttributeFeatures (there are no attributes which apply to stmt & decl like there are for type & decl, which is why that assert exists). I would also guess that diagnoseAppertainsTo would already handle the diagnostic for this case (but if not, that generated code may need updating as well). ~Aaron > let Documentation = [FallthroughDocs]; > } > > Index: include/clang/Basic/DiagnosticSemaKinds.td > =================================================================== > --- include/clang/Basic/DiagnosticSemaKinds.td > +++ include/clang/Basic/DiagnosticSemaKinds.td > @@ -2264,7 +2264,7 @@ > "Objective-C instance methods|init methods of interface or class extension > declarations|" > "variables, functions and classes|Objective-C protocols|" > "functions and global variables|structs, unions, and typedefs|structs and > typedefs|" > - "interface or protocol declarations|kernel functions}1">, > + "interface or protocol declarations|kernel functions|empty statements}1">, > InGroup<IgnoredAttributes>; > def err_attribute_wrong_decl_type : > Error<warn_attribute_wrong_decl_type.Text>; > def warn_type_attribute_wrong_type : Warning< > Index: include/clang/Sema/AttributeList.h > =================================================================== > --- include/clang/Sema/AttributeList.h > +++ include/clang/Sema/AttributeList.h > @@ -846,7 +846,8 @@ > ExpectedStructOrUnionOrTypedef, > ExpectedStructOrTypedef, > ExpectedObjectiveCInterfaceOrProtocol, > - ExpectedKernelFunction > + ExpectedKernelFunction, > + ExpectedEmptyStatement > }; > > } // end namespace clang > Index: test/SemaCXX/switch-implicit-fallthrough.cpp > =================================================================== > --- test/SemaCXX/switch-implicit-fallthrough.cpp > +++ test/SemaCXX/switch-implicit-fallthrough.cpp > @@ -1,6 +1,5 @@ > // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough %s > > - > int fallthrough(int n) { > switch (n / 10) { > if (n - 1) { > @@ -300,3 +299,8 @@ > } > return n; > } > + > +[[clang::fallthrough]] int a; // expected-error {{'fallthrough' attribute > only applies to empty statements}} > +[[clang::fallthrough]] int f(); // expected-error {{'fallthrough' attribute > only applies to empty statements}} > +void g([[clang::fallthrough]] int p); // expected-error {{'fallthrough' > attribute only applies to empty statements}} > +struct [[clang::fallthrough]] S; // expected-error {{'fallthrough' attribute > only applies to empty statements}} > > EMAIL PREFERENCES > http://reviews.llvm.org/settings/panel/emailpreferences/ _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
