On Thu, Jul 9, 2015 at 7:52 AM, Aaron Ballman <aa...@aaronballman.com> wrote:
> On Thu, Jul 9, 2015 at 12:22 AM, Hubert Tong > <hubert.reinterpretc...@gmail.com> 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? > 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). > > namespace A { > template<typename T> concept bool D1() { return true; } > } > > Either way, this is a test that should be added. > Agreed. > > ~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 > > cfe-commits@cs.uiuc.edu > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits