On Tue, Aug 11, 2015 at 4:54 PM, Richard Smith <rich...@metafoo.co.uk> wrote:
> On Tue, Aug 11, 2015 at 2:46 PM, Nathan Wilson <nwilso...@gmail.com> > wrote: > >> 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? >> > > Maybe err_concept_wrong_decl_kind? > I'd be okay with that. Is okay to have two different tags with the same text? We'd use err_concept_decl_non_template in HandleDeclarator when we encounter a non-template when `concept` is specified, and we could use err_concept_wrong_decl_kind for the checks in this Patch. > > >> 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
