[PATCH] D11027: [CONCEPTS] Creating Diagnostics for ill-formed function concept declaration

[Nathan Wilson]

nwilson added inline comments.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1964
@@ +1963,3 @@
+def err_concept_decls_may_only_appear_in_global_scope : Error<
+  "function and variable concept declarations may only appear in global scope">;
+def err_function_concept_not_defined : Error<
----------------
hubert.reinterpretcast wrote:
> Perhaps s/function and variable concept/concept/; see Aaron's response for further changes.
to make mention of it here, Aaron suggested having, "concept declarations may only appear in namespace scope" (since there aren't any concept declarations other than function and variable ones).

Is there any other opinion or preference on the phrasing?



================
Comment at: lib/Sema/SemaDecl.cpp:4887
@@ +4886,3 @@
+      Diag(D.getIdentifierLoc(),
+	   diag::err_concept_decls_may_only_appear_in_global_scope);
+      return nullptr;
----------------
hubert.reinterpretcast wrote:
> There seems to be an indenting issue caused by a tab character here.
Sorry. Will fix.

================
Comment at: lib/Sema/SemaDecl.cpp:7455
@@ +7454,3 @@
+    if (isConcept) {
+      // We are checking that the function concept declaration is a definition
+      if (!D.isFunctionDefinition()) {
----------------
hubert.reinterpretcast wrote:
> Since the quote from the relevant part of the Standard has been moved elsewhere, it makes sense to quote it again up to the "function template" part.
That makes sense. I'll put it back in.

================
Comment at: test/SemaCXX/cxx-concept-declaration.cpp:7
@@ +6,2 @@
+  template<typename T> concept bool D1() { return true; } // expected-error {{function and variable concept declarations may only appear in global scope}}
+};
----------------
hubert.reinterpretcast wrote:
> Since we now cover variable cases for the scope check as well, we should have a test for it.
Will do.


http://reviews.llvm.org/D11027