[PATCH] D13357: [Concepts] Add diagnostic; specializations of variable and function concepts

[Richard Smith via cfe-commits]

On Wed, Oct 7, 2015 at 2:59 PM, Richard Smith via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> rsmith added inline comments.
>
> ================
> Comment at: lib/Sema/SemaDecl.cpp:5902-5915
> @@ -5901,1 +5901,16 @@
> +
> +      // C++ Concepts TS [dcl.spec.concept]p7: A program shall not
> declare [...]
> +      //  an explicit specialization, or a partial specialization of a
> concept
> +      // definition
> +      if (IsVariableTemplateSpecialization && !IsPartialSpecialization) {
> +        Diag(NewVD->getLocation(), diag::err_concept_decl_specialized)
> +          << 0 << 1;
> +        NewVD->setInvalidDecl(true);
> +      }
> +
> +      if (IsVariableTemplateSpecialization && IsPartialSpecialization) {
> +        Diag(NewVD->getLocation(), diag::err_concept_decl_specialized)
> +          << 0 << 2;
> +        NewVD->setInvalidDecl(true);
> +      }
>      }
> ----------------
> Maybe combine these into a single check, and use `<<
> (IsPartialSpecialization ? 2 : 1)` for the specialization kind.
>
> ================
> Comment at: lib/Sema/SemaDecl.cpp:7562
> @@ -7546,1 +7561,3 @@
>      if (isConcept) {
> +      // This is a function concept
> +      NewFD->setConcept(true);
> ----------------
> Missing period.
>
> ================
> Comment at: lib/Sema/SemaDecl.cpp:7888
> @@ -7863,1 +7887,3 @@
> +
> +      if (NewFD->isInvalidDecl() && !NewFD->isConcept()) {
>          HasExplicitTemplateArgs = false;
> ----------------
> I'm not convinced this is going to work: if the declaration is invalid
> because it's declared `concept` /and/ for another reason, we presumably
> want to use the same error recovery path as before.
>
> ================
> Comment at: test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.concept/p7.cpp:1
> @@ +1,2 @@
> +// RUN:  %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s
> +
> ----------------
> This file is in the wrong directory; the relevant section is
> [dcl.spec.concept], not [dcl.concept].
>
> ================
> Comment at: test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.concept/p7.cpp:4
> @@ +3,3 @@
> +template<typename T> concept bool VCEI { true };
> +template concept bool VCEI<int>; // expected-error {{variable concept
> cannot be explicitly instantiated}}
> +
> ----------------
> hubert.reinterpretcast wrote:
> > This is not a declaration (never mind definition) of a function template
> or variable template, but of a specialization. Presumably, this violates
> [dcl.spec.concept]p1. Perhaps a test for p7 should omit the concept
> specifier? The same logic may apply to the explicit specialization cases.
> Right, there are two different ways things can go wrong here:
>
> 1) `concept` is specified on a non-template (such as in an explicit
> specialization or explicit instantiation declaration) or a variable
> template partial specialization
> 2) an explicit or partial specialization or an explicit instantiation
> declares (*without* the `concept` keyword) a specialization of a template
> that was declared as a concept.
>
> This patch is only checking for the former case, whereas the rule in
> [dcl.spec.concept]p7 says that the latter case is ill-formed. So these
> tests are at least in the wrong file. To check for violations of p7, you'll
> need to look at whether the template's pattern is marked as being a concept.
>
> I'm also not sure where we say that a partial specialization of a variable
> template cannot be declared `concept`; that seems to be a defect in the TS
> wording.


Hah, I see Hubert also found this and filed a DR last week :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151007/da472433/attachment.html>