On Wed, Oct 7, 2015 at 2:59 PM, Richard Smith via cfe-commits < cfe-commits@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 :)
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits