>From Aaron's description of the user experience, I think the err_concept_decl_non_template message text is good (although err_concept_decl_non_template might need to be renamed).
-- HT On Tue, Aug 11, 2015 at 4:01 PM, Aaron Ballman <aaron.ball...@gmail.com> wrote: > On Tue, Aug 11, 2015 at 3:36 PM, Nathan Wilson <nwilso...@gmail.com> > wrote: > > > > > > On Tue, Aug 11, 2015 at 8:54 AM, Aaron Ballman <aaron.ball...@gmail.com> > > wrote: > >> > >> On Mon, Aug 10, 2015 at 3:00 PM, Nathan Wilson <nwilso...@gmail.com> > >> wrote: > >> > nwilson created this revision. > >> > nwilson added reviewers: rsmith, hubert.reinterpretcast, fraggamuffin, > >> > faisalv, aaron.ballman. > >> > nwilson added a subscriber: cfe-commits. > >> > > >> > Adding check to emit diagnostic for invalid tag when concept is > >> > specified and associated tests. > >> > > >> > http://reviews.llvm.org/D11916 > >> > > >> > Files: > >> > include/clang/Basic/DiagnosticSemaKinds.td > >> > lib/Sema/SemaDecl.cpp > >> > test/SemaCXX/cxx-concept-declaration.cpp > >> > > >> > Index: test/SemaCXX/cxx-concept-declaration.cpp > >> > =================================================================== > >> > --- test/SemaCXX/cxx-concept-declaration.cpp > >> > +++ test/SemaCXX/cxx-concept-declaration.cpp > >> > @@ -23,3 +23,13 @@ > >> > template<typename T> > >> > concept bool D6; // expected-error {{variable concept declaration > must > >> > be initialized}} > >> > > >> > +// Tag > >> > +concept class CC1 {}; // expected-error {{class cannot be marked > >> > concept}} > >> > +concept struct CS1 {}; // expected-error {{struct cannot be marked > >> > concept}} > >> > +concept union CU1 {}; // expected-error {{union cannot be marked > >> > concept}} > >> > +concept enum CE1 {}; // expected-error {{enum cannot be marked > >> > concept}} > >> > +template <typename T> concept class TCC1 {}; // expected-error > {{class > >> > cannot be marked concept}} > >> > +template <typename T> concept struct TCS1 {}; // expected-error > >> > {{struct cannot be marked concept}} > >> > +template <typename T> concept union TCU1 {}; // expected-error > {{union > >> > cannot be marked concept}} > >> > + > >> > +extern concept bool ExtC; // expected-error {{'concept' can only > appear > >> > on the definition of a function template or variable template}} > >> > Index: lib/Sema/SemaDecl.cpp > >> > =================================================================== > >> > --- lib/Sema/SemaDecl.cpp > >> > +++ lib/Sema/SemaDecl.cpp > >> > @@ -3662,6 +3662,19 @@ > >> > return TagD; > >> > } > >> > > >> > + if (DS.isConceptSpecified()) { > >> > + // C++ Concepts TS [dcl.spec.concept]p1: A concept definition > >> > refers to > >> > + // either a function concept and its definition or a variable > >> > concept and > >> > + // its initializer. > >> > + if (Tag) > >> > + Diag(DS.getConceptSpecLoc(), diag::err_concept_tag) > >> > + << GetDiagnosticTypeSpecifierID(DS.getTypeSpecType()); > >> > + else > >> > + Diag(DS.getConceptSpecLoc(), > >> > diag::err_concept_decl_non_template); > >> > + // Don't emit warnings after this error. > >> > + return TagD; > >> > + } > >> > >> I'm not certain I understand why we need two different diagnostics for > >> this case. I think err_concept_decl_non_template is sufficiently clear > >> for both. > >> > >> ~Aaron > > > > > > This was based on how constexpr handles these checks. > > > > Perhaps someone can correct me if I'm wrong, but I believe the idea is > that > > when the `struct` tag exists, for example, we think the user meant to > > declare a `struct` and not the start of a `concept` declaration. So, the > > `concept` specifier would be erroneous and we try give a more helpful > > diagnostic. > > It could just be me, but I don't find the new diagnostic particularly > helpful. "foo cannot be marked concept" tells me why I have an error > but does not tell me what to do about it. "'concept' can only appear > on the definition of a function template or variable template" tells > me what I need to do to not have the error in the first place, as well > as why I have the error. > > However, others may have differing opinions on the subject. > > ~Aaron > > > > > I would need to add/fix the test case for this, but I tend to think the > > declaration such as `concept bool;` could be the users intention to try > to > > create a `concept` declaration which is where the > > err_concept_decl_non_template comes in. > > > >> > >> > >> > + > >> > DiagnoseFunctionSpecifiers(DS); > >> > > >> > if (DS.isFriendSpecified()) { > >> > Index: include/clang/Basic/DiagnosticSemaKinds.td > >> > =================================================================== > >> > --- include/clang/Basic/DiagnosticSemaKinds.td > >> > +++ include/clang/Basic/DiagnosticSemaKinds.td > >> > @@ -1973,6 +1973,8 @@ > >> > "function concept declaration must be a definition">; > >> > def err_var_concept_not_initialized : Error< > >> > "variable concept declaration must be initialized">; > >> > +def err_concept_tag : Error< > >> > + "%select{class|struct|interface|union|enum}0 cannot be marked > >> > concept">; > >> > > >> > // C++11 char16_t/char32_t > >> > def warn_cxx98_compat_unicode_type : Warning< > >> > > >> > > > > > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
