<p dir="ltr"><br>
On Jul 9, 2015 8:21 AM, "Aaron Ballman" <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br>
><br>
> On Thu, Jul 9, 2015 at 9:17 AM, Hubert Tong<br>
> <<a href="mailto:hubert.reinterpretcast@gmail.com">hubert.reinterpretcast@gmail.com</a>> wrote:<br>
> > On Thu, Jul 9, 2015 at 7:52 AM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>><br>
> > wrote:<br>
> >><br>
> >> On Thu, Jul 9, 2015 at 12:22 AM, Hubert Tong<br>
> >> <<a href="mailto:hubert.reinterpretcast@gmail.com">hubert.reinterpretcast@gmail.com</a>> wrote:<br>
> >> > hubert.reinterpretcast added inline comments.<br>
> >> ><br>
> >> > ================<br>
> >> > Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1964<br>
> >> > @@ +1963,3 @@<br>
> >> > +def err_invalid_concept_scope : Error<<br>
> >> > +  "concept declaration required to be in namespace scope">;<br>
> >> > +def err_function_concept_not_defined : Error<<br>
> >> > ----------------<br>
> >> > This error is similar to<br>
> >> > `err_objc_decls_may_only_appear_in_global_scope` below, so perhaps it could<br>
> >> > be expressed in a similar manner.<br>
> >><br>
> >> Can't concepts be declared in a namespace scope that's not the global<br>
> >> namespace scope?<br>
> ><br>
> > Yes. I should have been more specific. The similarity I was referring to did<br>
> > not extend to the "global scope" part. Note that I added my comment to the<br>
> > message text (not the check, which does allow namespaces other than the<br>
> > global one).<br>
><br>
> I mentioned it because the current patch has:<br>
><br>
> +def err_concept_decls_may_only_appear_in_global_scope : Error<<br>
> +  "function and variable concept declarations may only appear in<br>
> global scope">;<br>
><br>
> which is a misleading diagnostic (both the identifier and the<br>
> wording). I probably should have attributed that to the current patch<br>
> and not your comment, however. ;-)</p>
<p dir="ltr">Yeah, sorry, probably shouldn't have tried to do it so late at night...</p>
<p dir="ltr">Would this wording suffice?:</p>
<p dir="ltr">def err_concept_decls_may_only_appear_in_namespace_scope : Error<<br>
"function and variable concept declarations may only appear in namespace scope">;</p>
<p dir="ltr">><br>
> ~Aaron<br>
><br>
> ><br>
> >><br>
> >><br>
> >> namespace A {<br>
> >>   template<typename T> concept bool D1() { return true; }<br>
> >> }<br>
> >></p>
<p dir="ltr">Makes sense. I'll add that. Thanks!</p>
<p dir="ltr">> >><br>
> >> Either way, this is a test that should be added.<br>
> ><br>
> > Agreed.<br>
> ><br>
> >><br>
> >><br>
> >> ~Aaron<br>
> >><br>
> >> ><br>
> >> > ================<br>
> >> > Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1966<br>
> >> > @@ +1965,3 @@<br>
> >> > +def err_function_concept_not_defined : Error<<br>
> >> > +  "function concept requires definition">;<br>
> >> > +<br>
> >> > ----------------<br>
> >> > Perhaps: "can only declare a function concept with its definition".<br>
> >> ><br>
> >> > The existing wording seems to imply that an earlier definition (of just<br>
> >> > one somewhere) is sufficient and that function concepts may be redeclared.<br>
> >> ><br>
> >> > ================<br>
> >> > Comment at: lib/Sema/SemaDecl.cpp:7453<br>
> >> > @@ +7452,3 @@<br>
> >> > +      // C++ Concepts TS [dcl.spec.concept]p1: A concept definition<br>
> >> > refers to a<br>
> >> > +      // function concept and its definition<br>
> >> > +      if (!D.isFunctionDefinition()) {<br>
> >> > ----------------<br>
> >> > The quoted text is a definition of the term "concept definition". The<br>
> >> > diagnosable rule is established by the quote on line 7444. So there probably<br>
> >> > should be a comment here to the effect of "we are checking that the<br>
> >> > declaration is indeed a definition".<br>
> >> ><br>
> >> > ================<br>
> >> > Comment at: lib/Sema/SemaDecl.cpp:7460<br>
> >> > @@ +7459,3 @@<br>
> >> > +<br>
> >> > +      // C++ Concepts TS [dcl.spec.concept]p2: Every concepts<br>
> >> > definition is<br>
> >> > +      // implicity defined to be a constexpr declaration (implicitly<br>
> >> > inline)<br>
> >> > ----------------<br>
> >> > Typo: s/concepts/concept/<br>
> >> ><br>
> >> > ================<br>
> >> > Comment at: test/SemaCXX/cxx-concept-declaration.cpp:3<br>
> >> > @@ +2,3 @@<br>
> >> > +<br>
> >> > +template<typename T> concept bool C1 = true;<br>
> >> > +<br>
> >> > ----------------<br>
> >> > I'm not sure why this is part of the current patch.<br>
> >> ><br>
> >> > ================<br>
> >> > Comment at: test/SemaCXX/cxx-concept-declaration.cpp:5<br>
> >> > @@ +4,3 @@<br>
> >> > +<br>
> >> > +template<typename T> concept decltype(!0) C2 = true;<br>
> >> > +<br>
> >> > ----------------<br>
> >> > Same comment as on line 3.<br>
> >> ><br>
> >> ><br>
> >> > <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D11027&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=9gS4Ew7LhedtJmt6cJGswmEhg_WHfumYM-cUFPu0KpI&s=ReTMpg3qaPbGp1UtEbrVoZRgiRpGYfFN1ZUOl5HzB-I&e=">http://reviews.llvm.org/D11027</a><br>
> >> ><br>
> >> ><br>
> >> ><br>
> >> ><br>
> >> > _______________________________________________<br>
> >> > cfe-commits mailing list<br>
> >> > <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> >> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
> ><br>
> ><br>
</p>