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

[Aaron Ballman]

Tiny nits, otherwise LGTM, but please wait for Richard to also sign off.

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.

> +      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"?

> +
>  // 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
>