nwilson 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< ---------------- hubert.reinterpretcast wrote: > 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. Sure, change it to something similar.
================ Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1966 @@ +1965,3 @@ +def err_function_concept_not_defined : Error< + "function concept requires definition">; + ---------------- hubert.reinterpretcast wrote: > 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. Makes sense. I'll change the wording. ================ Comment at: lib/Sema/SemaDecl.cpp:7447 @@ +7446,3 @@ + // template, declared in namespace scope + if (!NewFD->getParent()->getRedeclContext()->isFileContext()) { + Diag(D.getIdentifierLoc(), diag::err_invalid_concept_scope); ---------------- Any thoughts about moving this check? I think it can be moved above to inside HandleDeclarator before we look at acting on a function or variable declarator since this check is applicable to both a function and variable concept Something like: if (!D.getContext() == FileContext) { Diag(...) } ================ 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()) { ---------------- hubert.reinterpretcast wrote: > 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". That makes sense. I'll make the change. ================ Comment at: test/SemaCXX/cxx-concept-declaration.cpp:5 @@ +4,3 @@ + +template<typename T> concept decltype(!0) C2 = true; + ---------------- hubert.reinterpretcast wrote: > Same comment as on line 3. I'll take them out since they're not necessary - I just wanted a couple of tests that would be pass, but yeah, it's unnecessary. http://reviews.llvm.org/D11027 _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits