On Thu, Jul 9, 2015 at 12:22 AM, Hubert Tong
<[email protected]> wrote:
> hubert.reinterpretcast added inline comments.
>
> ================
> Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1964
> @@ +1963,3 @@
> +def err_invalid_concept_scope : Error<
> + "concept declaration required to be in namespace scope">;
> +def err_function_concept_not_defined : Error<
> ----------------
> 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.
Can't concepts be declared in a namespace scope that's not the global
namespace scope?
namespace A {
template<typename T> concept bool D1() { return true; }
}
Either way, this is a test that should be added.
~Aaron
>
> ================
> Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1966
> @@ +1965,3 @@
> +def err_function_concept_not_defined : Error<
> + "function concept requires definition">;
> +
> ----------------
> Perhaps: "can only declare a function concept with its definition".
>
> The existing wording seems to imply that an earlier definition (of just one
> somewhere) is sufficient and that function concepts may be redeclared.
>
> ================
> Comment at: lib/Sema/SemaDecl.cpp:7453
> @@ +7452,3 @@
> + // C++ Concepts TS [dcl.spec.concept]p1: A concept definition refers
> to a
> + // function concept and its definition
> + if (!D.isFunctionDefinition()) {
> ----------------
> 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".
>
> ================
> Comment at: lib/Sema/SemaDecl.cpp:7460
> @@ +7459,3 @@
> +
> + // C++ Concepts TS [dcl.spec.concept]p2: Every concepts definition is
> + // implicity defined to be a constexpr declaration (implicitly inline)
> ----------------
> Typo: s/concepts/concept/
>
> ================
> Comment at: test/SemaCXX/cxx-concept-declaration.cpp:3
> @@ +2,3 @@
> +
> +template<typename T> concept bool C1 = true;
> +
> ----------------
> I'm not sure why this is part of the current patch.
>
> ================
> Comment at: test/SemaCXX/cxx-concept-declaration.cpp:5
> @@ +4,3 @@
> +
> +template<typename T> concept decltype(!0) C2 = true;
> +
> ----------------
> Same comment as on line 3.
>
>
> http://reviews.llvm.org/D11027
>
>
>
>
> _______________________________________________
> cfe-commits mailing list
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
