[CONCEPTS] Add diagnostics: non-defining function; non-namespace scope

[nwilson]

Summary

Create diagnostic for function concept declaration which is not a
definition.

Create diagnostic for concept declaration which isn't in namespace
scope.

Create associated tests.

[hubert.reinterpretcast]

include/clang/Basic/DiagnosticSemaKinds.td

1964

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.

1966

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.

lib/Sema/SemaDecl.cpp

7464

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".

7471

Typo: s/concepts/concept/

test/SemaCXX/cxx-concept-declaration.cpp

4

I'm not sure why this is part of the current patch.

6

Same comment as on line 3.

[nwilson]

include/clang/Basic/DiagnosticSemaKinds.td

1964

Sure, change it to something similar.

1966

Makes sense. I'll change the wording.

lib/Sema/SemaDecl.cpp

7458

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(...)

}

7464

That makes sense. I'll make the change.

test/SemaCXX/cxx-concept-declaration.cpp

6

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.

[hubert.reinterpretcast]

lib/Sema/SemaDecl.cpp

7458

You would need to use DC. The DeclContext is also adjusted "later" by adjustContextForLocalExternDecl in each of the function and variable cases. I'm not sure what happens with friend declarations. In this case, I think using DC before the adjustments makes sense (since we cannot declare a concept in block scope or as a friend).

[nwilson]

Addressing comments -
Changing diagnostic name and update wording.
Move check for concept scope being in namespace scope to HandleDeclarator.
Update comment's wording
Remove some unnecessary tests.

[hubert.reinterpretcast]

include/clang/Basic/DiagnosticSemaKinds.td

1964

Perhaps s/function and variable concept/concept/; see Aaron's response for further changes.

lib/Sema/SemaDecl.cpp

4887

There seems to be an indenting issue caused by a tab character here.

7455

Since the quote from the relevant part of the Standard has been moved elsewhere, it makes sense to quote it again up to the "function template" part.

test/SemaCXX/cxx-concept-declaration.cpp

3

Aaron suggested that we have acceptance testing for concepts in a namespace scope other than the global one.

8

Since we now cover variable cases for the scope check as well, we should have a test for it.

[nwilson]

include/clang/Basic/DiagnosticSemaKinds.td

1964

to make mention of it here, Aaron suggested having, "concept declarations may only appear in namespace scope" (since there aren't any concept declarations other than function and variable ones).

Is there any other opinion or preference on the phrasing?

lib/Sema/SemaDecl.cpp

4887

Sorry. Will fix.

7455

That makes sense. I'll put it back in.

test/SemaCXX/cxx-concept-declaration.cpp

8

Will do.

[hubert.reinterpretcast]

include/clang/Basic/DiagnosticSemaKinds.td

1964

Apparently, Aaron and I are of the same mind on the phrasing. :)

[nwilson]

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

[nwilson]

test/SemaCXX/cxx-concept-declaration.cpp

4

Please let me know if feel any tests are missing at this point.

[nwilson]

As suggested by Aaron, modify phrasing of err_function_concept_not_defined
Remove redundant comment for concept declaration being definition

[nwilson]

Ping

[rsmith]

include/clang/Basic/DiagnosticSemaKinds.td

1965–1966

Does this not also apply to variable concepts? For instance

template<typename T> extern concept bool b;

[nwilson]

include/clang/Basic/DiagnosticSemaKinds.td

1965–1966

Could we create a separate diagnostic for that? i.e.:

def err_variable_concept_not_intialized : Error<

"variable concept declaration requires initialization">;

[rsmith]

include/clang/Basic/DiagnosticSemaKinds.td

1965–1966

That seems fine to me.

[hubert.reinterpretcast]

It seems all of the raised issues have been resolved; LGTM.

include/clang/Basic/DiagnosticSemaKinds.td

 def note_non_literal_nontrivial_dtor : Note<
   "%0 is not literal because it has a non-trivial destructor">;
 def warn_private_extern : Warning<
   "use of __private_extern__ on a declaration may not produce external symbol "
   "private to the linkage unit and is deprecated">, InGroup<PrivateExtern>;
 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<
+  "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">,
   InGroup<CXX98Compat>, DefaultIgnore;
  
 // Objective-C++
 def err_objc_decls_may_only_appear_in_global_scope : Error<
   "Objective-C declarations may only appear in global scope">;

lib/Sema/SemaDecl.cpp

if (Previous.isSingleTagDecl() &&
       D.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_typedef)
     Previous.clear();
 
   // Check that there are no default arguments other than in the parameters
   // of a function declaration (C++ only).
   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
+    if (!DC->getRedeclContext()->isFileContext()) {
+      Diag(D.getIdentifierLoc(),
+           diag::err_concept_decls_may_only_appear_in_namespace_scope);
+      return nullptr;
+    }
+  }
+
   NamedDecl *New;
 
   bool AddToScope = true;
   if (D.getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_typedef) {
     if (TemplateParamLists.size()) {
       Diag(D.getIdentifierLoc(), diag::err_template_typedef);
       return nullptr;
     }
Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
   if (IsLocalExternDecl)
     NewFD->setLocalExternDecl();
 
   if (getLangOpts().CPlusPlus) {
     bool isInline = D.getDeclSpec().isInlineSpecified();
     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
       //   A function can be defined in a friend declaration of a
       //   class . . . . Such a function is implicitly inline.
       NewFD->setImplicitlyInline();
     }
 
if (isConstexpr) {
 
       // C++11 [dcl.constexpr]p3: functions declared constexpr are required to
       // be either constructors or to return a literal type. Therefore,
       // destructors cannot be declared constexpr.
       if (isa<CXXDestructorDecl>(NewFD))
         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...
+      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) {
         SourceLocation ModulePrivateLoc
           = D.getDeclSpec().getModulePrivateSpecLoc();
         Diag(ModulePrivateLoc, diag::err_module_private_specialization)
           << 0
           << FixItHint::CreateRemoval(ModulePrivateLoc);
       } else {
         NewFD->setModulePrivate();
         if (FunctionTemplate)
           FunctionTemplate->setModulePrivate();

test/SemaCXX/cxx-concept-declaration.cpp

+// 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 {{function concept declaration must be a 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}}
+};