<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jul 9, 2015 at 7:52 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">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 `err_objc_decls_may_only_appear_in_global_scope` below, so perhaps it could be expressed in a similar manner.<br>
<br>
</span>Can't concepts be declared in a namespace scope that's not the global<br>
namespace scope?<br></blockquote><div>Yes. I should have been more specific. The similarity I was referring to did not extend to the "global scope" part. Note that I added my comment to the message text (not the check, which does allow namespaces other than the global one).<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
namespace A {<br>
<span class="">  template<typename T> concept bool D1() { return true; }<br>
}<br></span></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
<br>
</span>Either way, this is a test that should be added.<br></blockquote><div>Agreed.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
~Aaron<br>
<div><div class="h5"><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 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 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 diagnosable rule is established by the quote on line 7444. So there probably should be a comment here to the effect of "we are checking that the 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 definition is<br>
> +      // implicity defined to be a constexpr declaration (implicitly 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=ObyluusbJAovJ96HuuzuOC2WdI7igaw63YFjGS99FU0&s=Mx3a-n7meYKmjbdEegGa28tqCKD9mKTOqwcSYMojN-Q&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D11027</a><br>
><br>
><br>
><br>
><br>
</div></div>> _______________________________________________<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" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>