Okay, I'll make the change. Hmm, do you guys have any suggestions as far as renaming err_concept_decl_non_template? How about err_concept_specifier_non_template?
We'd have to keep in mind that err_concept_decl_non_template is also used outside of this check, i.e. `concept bool = true`. On Tue, Aug 11, 2015 at 4:19 PM, Hubert Tong < hubert.reinterpretc...@gmail.com> wrote: > 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
