<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Oct 7, 2015 at 2:59 PM, Richard Smith via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">rsmith added inline comments.<br>
<br>
================<br>
Comment at: lib/Sema/SemaDecl.cpp:5902-5915<br>
@@ -5901,1 +5901,16 @@<br>
+<br>
+      // C++ Concepts TS [dcl.spec.concept]p7: A program shall not declare [...]<br>
+      //  an explicit specialization, or a partial specialization of a concept<br>
+      // definition<br>
+      if (IsVariableTemplateSpecialization && !IsPartialSpecialization) {<br>
+        Diag(NewVD->getLocation(), diag::err_concept_decl_specialized)<br>
+          << 0 << 1;<br>
+        NewVD->setInvalidDecl(true);<br>
+      }<br>
+<br>
+      if (IsVariableTemplateSpecialization && IsPartialSpecialization) {<br>
+        Diag(NewVD->getLocation(), diag::err_concept_decl_specialized)<br>
+          << 0 << 2;<br>
+        NewVD->setInvalidDecl(true);<br>
+      }<br>
     }<br>
----------------<br>
Maybe combine these into a single check, and use `<< (IsPartialSpecialization ? 2 : 1)` for the specialization kind.<br>
<br>
================<br>
Comment at: lib/Sema/SemaDecl.cpp:7562<br>
@@ -7546,1 +7561,3 @@<br>
     if (isConcept) {<br>
+      // This is a function concept<br>
+      NewFD->setConcept(true);<br>
----------------<br>
Missing period.<br>
<br>
================<br>
Comment at: lib/Sema/SemaDecl.cpp:7888<br>
@@ -7863,1 +7887,3 @@<br>
+<br>
+      if (NewFD->isInvalidDecl() && !NewFD->isConcept()) {<br>
         HasExplicitTemplateArgs = false;<br>
----------------<br>
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.<br>
<br>
================<br>
Comment at: test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.concept/p7.cpp:1<br>
@@ +1,2 @@<br>
+// RUN:  %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s<br>
+<br>
----------------<br>
This file is in the wrong directory; the relevant section is [dcl.spec.concept], not [dcl.concept].<br>
<span class=""><br>
================<br>
Comment at: test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.concept/p7.cpp:4<br>
@@ +3,3 @@<br>
+template<typename T> concept bool VCEI { true };<br>
+template concept bool VCEI<int>; // expected-error {{variable concept cannot be explicitly instantiated}}<br>
+<br>
----------------<br>
</span><span class="">hubert.reinterpretcast wrote:<br>
> 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.<br>
</span>Right, there are two different ways things can go wrong here:<br>
<br>
1) `concept` is specified on a non-template (such as in an explicit specialization or explicit instantiation declaration) or a variable template partial specialization<br>
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.<br>
<br>
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.<br>
<br>
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.</blockquote><div><br></div><div>Hah, I see Hubert also found this and filed a DR last week :) </div></div></div></div>