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

[Hubert Tong]

On Thu, Jul 9, 2015 at 7:52 AM, Aaron Ballman <aaron at aaronballman.com>
wrote:

> On Thu, Jul 9, 2015 at 12:22 AM, Hubert Tong
> <hubert.reinterpretcast at 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 at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150709/787505b8/attachment.html>