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

Aaron Ballman aaron at aaronballman.com
Fri Jul 10 08:02:50 PDT 2015


On Fri, Jul 10, 2015 at 11:01 AM, Nathan Wilson <nwilson20 at gmail.com> wrote:
>
>
> On Fri, Jul 10, 2015 at 9:44 AM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> On Fri, Jul 10, 2015 at 10:34 AM, Nathan Wilson <nwilson20 at gmail.com>
>> wrote:
>> >
>> >
>> > On Fri, Jul 10, 2015 at 8:52 AM, Hubert Tong
>> > <hubert.reinterpretcast at gmail.com> wrote:
>> >>
>> >> On Fri, Jul 10, 2015 at 8:30 AM, Aaron Ballman <aaron at aaronballman.com>
>> >> wrote:
>> >>>
>> >>> Tiny nits, otherwise LGTM, but please wait for Richard to also sign
>> >>> off.
>> >>
>> >> Same from my end.
>> >>
>> >>>
>> >>>
>> >>> On Fri, Jul 10, 2015 at 12:21 AM, Nathan Wilson <nwilson20 at gmail.com>
>> >>> wrote:
>> >>> > nwilson updated this revision to Diff 29426.
>> >>> > nwilson added a comment.
>> >>> >
>> >>> > Update phrasing for concept declaration scope diagnostic
>> >>> > Fix indentation issue
>> >>> > Update comments for function declaration being a definition
>> >>> > Adding acceptance tests for concepts in namespace scope and adding
>> >>> > diagnostic test for variable concept not being in namespace scope
>> >>> >
>> >>> >
>> >>> > http://reviews.llvm.org/D11027
>> >>> >
>> >>> > Files:
>> >>> >   include/clang/Basic/DiagnosticSemaKinds.td
>> >>> >   lib/Sema/SemaDecl.cpp
>> >>> >   test/SemaCXX/cxx-concept-declaration.cpp
>> >>> >
>> >>> > Index: test/SemaCXX/cxx-concept-declaration.cpp
>> >>> > ===================================================================
>> >>> > --- /dev/null
>> >>> > +++ test/SemaCXX/cxx-concept-declaration.cpp
>> >>> > @@ -0,0 +1,17 @@
>> >>> > +// RUN:  %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s
>> >>> > +
>> >>> > +namespace A {
>> >>> > +  template<typename T> concept bool C1() { return true; }
>> >>> > +
>> >>> > +  template<typename T> concept bool C2 = true;
>> >>> > +}
>> >>> > +
>> >>> > +template<typename T> concept bool D1(); // expected-error {{can
>> >>> > only
>> >>> > declare a function concept with its definition}}
>> >>> > +
>> >>> > +struct B {
>> >>> > +  template<typename T> concept bool D2() { return true; } //
>> >>> > expected-error {{concept declarations may only appear in namespace
>> >>> > scope}}
>> >>> > +};
>> >>> > +
>> >>> > +struct C {
>> >>> > +  template<typename T> static concept bool D3 = true; //
>> >>> > expected-error {{concept declarations may only appear in namespace
>> >>> > scope}}
>> >>> > +};
>> >>> > Index: lib/Sema/SemaDecl.cpp
>> >>> > ===================================================================
>> >>> > --- lib/Sema/SemaDecl.cpp
>> >>> > +++ lib/Sema/SemaDecl.cpp
>> >>> > @@ -4878,6 +4878,17 @@
>> >>> >    if (getLangOpts().CPlusPlus)
>> >>> >      CheckExtraCXXDefaultArguments(D);
>> >>> >
>> >>> > +  if (D.getDeclSpec().isConceptSpecified()) {
>> >>> > +    // C++ Concepts TS [dcl.spec.concept]p1: The concept specifier
>> >>> > shall be
>> >>> > +    // applied only to the definition of a function template or
>> >>> > variable
>> >>> > +    // template, declared in namespace scope
>> >>>
>> >>> Missing a period.
>> >>>
>> >>> > +    if (!DC->getRedeclContext()->isFileContext()) {
>> >>> > +      Diag(D.getIdentifierLoc(),
>> >>> > +
>> >>> > diag::err_concept_decls_may_only_appear_in_namespace_scope);
>> >>> > +      return nullptr;
>> >>> > +    }
>> >>> > +  }
>> >>> > +
>> >>> >    NamedDecl *New;
>> >>> >
>> >>> >    bool AddToScope = true;
>> >>> > @@ -7223,6 +7234,7 @@
>> >>> >      bool isVirtual = D.getDeclSpec().isVirtualSpecified();
>> >>> >      bool isExplicit = D.getDeclSpec().isExplicitSpecified();
>> >>> >      bool isConstexpr = D.getDeclSpec().isConstexprSpecified();
>> >>> > +    bool isConcept = D.getDeclSpec().isConceptSpecified();
>> >>> >      isFriend = D.getDeclSpec().isFriendSpecified();
>> >>> >      if (isFriend && !isInline && D.isFunctionDefinition()) {
>> >>> >        // C++ [class.friend]p5
>> >>> > @@ -7439,6 +7451,21 @@
>> >>> >          Diag(D.getDeclSpec().getConstexprSpecLoc(),
>> >>> > diag::err_constexpr_dtor);
>> >>> >      }
>> >>> >
>> >>> > +    if (isConcept) {
>> >>> > +      // C++ Concepts TS [dcl.spec.concept]p1: The concept
>> >>> > specifier
>> >>> > shall be
>> >>> > +      // applied only to the definition of a function template
>> >>> > +      // (we are checking that the declaration is indeed a
>> >>> > definition)
>> >>>
>> >>> Missing a period.
>> >>
>> >> I think ellipsis is appropriate. Also, the parenthesized part is
>> >> redundant
>> >> now.
>> >
>> >
>> > Yeah, I was attempting to be explicit, but I guess the code does that. I
>> > can
>> > make the change.
>> >>
>> >>
>> >>>
>> >>>
>> >>> > +      if (!D.isFunctionDefinition()) {
>> >>> > +        Diag(D.getDeclSpec().getConceptSpecLoc(),
>> >>> > +             diag::err_function_concept_not_defined);
>> >>> > +        NewFD->setInvalidDecl();
>> >>> > +      }
>> >>> > +
>> >>> > +      // C++ Concepts TS [dcl.spec.concept]p2: Every concept
>> >>> > definition is
>> >>> > +      // implicity defined to be a constexpr declaration
>> >>> > (implicitly
>> >>> > inline)
>> >>> > +      NewFD->setImplicitlyInline();
>> >>> > +    }
>> >>> > +
>> >>> >      // If __module_private__ was specified, mark the function
>> >>> > accordingly.
>> >>> >      if (D.getDeclSpec().isModulePrivateSpecified()) {
>> >>> >        if (isFunctionTemplateSpecialization) {
>> >>> > Index: include/clang/Basic/DiagnosticSemaKinds.td
>> >>> > ===================================================================
>> >>> > --- include/clang/Basic/DiagnosticSemaKinds.td
>> >>> > +++ include/clang/Basic/DiagnosticSemaKinds.td
>> >>> > @@ -1959,6 +1959,12 @@
>> >>> >  def note_private_extern : Note<
>> >>> >    "use __attribute__((visibility(\"hidden\"))) attribute instead">;
>> >>> >
>> >>> > +// C++ Concepts TS
>> >>> > +def err_concept_decls_may_only_appear_in_namespace_scope : Error<
>> >>> > +  "concept declarations may only appear in namespace scope">;
>> >>> > +def err_function_concept_not_defined : Error<
>> >>> > +  "can only declare a function concept with its definition">;
>> >>>
>> >>> This reads slightly strange to me. We have another diagnostic that we
>> >>> could steal wording from:
>> >>>
>> >>> def err_invalid_constexpr_var_decl : Error<
>> >>>   "constexpr variable declaration must be a definition">;
>> >>>
>> >>> So perhaps "function concept declaration must be a definition"?
>> >>
>> >> That sounds good.
>> >
>> >
>> > Okay, that works. Do you guys have any issue with the name:
>> > err_function_concept_not_defined
>> >
>> > Or, would something like this be more appropriate?:
>> > err_invalid_concept_function_decl
>>
>> I prefer err_invalid_concept_function_decl.
>
>
> Hmm, well, I'm leaning to what Hubert said about it being a bit too generic
> since there are other places where a name like this would be applicable
> (declared returning type not being bool, non empty parameter list, ...)

I can swing around to that viewpoint. :-)

~Aaron

>
>>
>> ~Aaron
>>
>> >
>> >>
>> >>
>> >>>
>> >>>
>> >>> > +
>> >>> >  // C++11 char16_t/char32_t
>> >>> >  def warn_cxx98_compat_unicode_type : Warning<
>> >>> >    "'%0' type specifier is incompatible with C++98">,
>> >>>
>> >>> ~Aaron
>> >>>
>> >>> >
>> >>> >
>> >>> >
>> >>> > _______________________________________________
>> >>> > cfe-commits mailing list
>> >>> > cfe-commits at cs.uiuc.edu
>> >>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> >>> >
>> >>
>> >>
>> >
>
>



More information about the cfe-commits mailing list