[PATCH] D11916: [CONCEPTS] Add diagnostic; invalid tag when concept specified

[Nathan Wilson via cfe-commits]

On Tue, Aug 11, 2015 at 8:54 AM, Aaron Ballman <aaron.ballman at gmail.com>
wrote:

> On Mon, Aug 10, 2015 at 3:00 PM, Nathan Wilson <nwilson20 at 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.

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<
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150811/5f0a07c4/attachment.html>