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