[Clang] Reword diagnostic for scope identifier with linkage

[junaire]

Summary

If the declaration of an identifier has block scope, and the identifier has
external or internal linkage, the declaration shall have no initializer for
the identifier.

Clang now gives a more suitable diagnosis for this case.
Fixes https://github.com/llvm/llvm-project/issues/57478

Signed-off-by: Jun Zhang <[e-mail]>

[junaire]

Simpify code a little bit.

[pmor13]

Note that this code:

int x;

void f(void)
{
    extern int x = 1;
}

needs to produce:

error: declaration of block scope identifier with external linkage shall have no initializer

So, err_block_extern_cant_init needs to be updated too (I think).

[junaire]

Update the existing diagnostic and its tests.

[pmor13]

Looks good.

[junaire]

Rebase

[junaire]

gentle ping :)

[aaron.ballman]

clang/include/clang/Basic/DiagnosticSemaKinds.td

5904–5905

I don't think we want to try to call out external vs internal linkage here, more on why below. Also, we don't usually use "shall" in diagnostic wording.

clang/lib/Sema/SemaDecl.cpp

12776–12786

12781–12782

clang/test/Sema/err-decl-block-extern-no-init.c

7

This example is why I think we should reword the diagnostic. As it stands, this diagnostic wording would be baffling to most users because they'd go "whaddya mean *INTERNAL* linkage, I said extern for a reason!" So using a diagnostic wording of "with linkage" sidesteps that -- what the linkage is doesn't matter for fixing the code, it's the initializer that's the issue.

Also, I don't know what it means for a block scope variable to have *internal* linkage instead of *no* linkage (and I'm not certain it's possible to observe the difference).

That said, I think this example should have a *second* diagnostic, which is a warning, letting the user know that the actual linkage does not match the specified linkage. (This should be done in a separate patch.)

[pmor13]

@aaron.ballman

block scope variable to have *internal* linkage instead of *no* linkage

static int x;

void f(void)
{
    extern int x;  // block scope, internal linkage 
}

[aaron.ballman]

In D133088#3782126, @pmor13 wrote:

@aaron.ballman

block scope variable to have *internal* linkage instead of *no* linkage

static int x;

void f(void)
{
    extern int x;  // block scope, internal linkage 
}

No, I understood that, I meant in terms of the semantics. I'm not 100% convinced there's a way to *observe* the difference between no and internal linkage, but I *think* you might be able to observe it regarding an inline function returning the address of an internal linkage variable. If it's internal linkage, every copy of the function shares the same object but if it had no linkage, the linker wouldn't have to collapse them all down to the same object (maybe?).

[junaire]

Try to address Aaron's comments.

[junaire]

Rebase, adjust commit message and release note.

[aaron.ballman]

LGTM, thank you!

clang/docs/ReleaseNotes.rst

 - Correctly diagnose a future keyword if it exist as a keyword in the higher
   language version and specifies in which version it will be a keyword. This
   supports both c and c++ language.
 - When diagnosing multi-level pack expansions of mismatched lengths, Clang will
   now, in most cases, be able to point to the relevant outer parameter.
 - no_sanitize("...") on a global variable for known but not relevant sanitizers
   is now just a warning. It now says that this will be ignored instead of
   incorrectly saying no_sanitize only applies to functions and methods.
+- Clang will now give a more suitale diagnostic for declaration of block
+  scope identifiers that have external/internal linkage that has an initializer.
+  Fixes `Issue 57478: <https://github.com/llvm/llvm-project/issues/57478>`_.
+
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------
 - It's now possible to set the crash diagnostics directory through
   the environment variable ``CLANG_CRASH_DIAGNOSTICS_DIR``.
   The ``-fcrash-diagnostics-dir`` flag takes precedence.
 - When using header modules, inclusion of a private header and violations of
   the `use-declaration rules

clang/include/clang/Basic/DiagnosticSemaKinds.td

def err_loader_uninitialized_trivial_ctor
     : Error<"variable with 'loader_uninitialized' attribute must have a "
             "trivial default constructor">;
 def err_loader_uninitialized_redeclaration
     : Error<"redeclaration cannot add 'loader_uninitialized' attribute">;
 def err_loader_uninitialized_extern_decl
     : Error<"variable %0 cannot be declared both 'extern' and with the "
             "'loader_uninitialized' attribute">;
 def err_block_extern_cant_init : Error<
-  "'extern' variable cannot have an initializer">;
+  "declaration of block scope identifier with linkage cannot have an initializer">;
 def warn_extern_init : Warning<"'extern' variable has an initializer">,
   InGroup<DiagGroup<"extern-initializer">>;
 def err_variable_object_no_init : Error<
   "variable-sized object may not be initialized">;
 def err_excess_initializers : Error<
   "excess elements in %select{array|vector|scalar|union|struct}0 initializer">;
 def ext_excess_initializers : ExtWarn<
   "excess elements in %select{array|vector|scalar|union|struct}0 initializer">,
   InGroup<ExcessInitializers>;

clang/lib/Sema/SemaDecl.cpp

void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
 
   // dllimport cannot be used on variable definitions.
   if (VDecl->hasAttr<DLLImportAttr>() && !VDecl->isStaticDataMember()) {
     Diag(VDecl->getLocation(), diag::err_attribute_dllimport_data_definition);
     VDecl->setInvalidDecl();
     return;
   }
 
+  // C99 6.7.8p5. If the declaration of an identifier has block scope, and
+  // the identifier has external or internal linkage, the declaration shall
+  // have no initializer for the identifier.
+  // C++14 [dcl.init]p5 is the same restriction for C++.
   if (VDecl->isLocalVarDecl() && VDecl->hasExternalStorage()) {
-    // C99 6.7.8p5. C++ has no such restriction, but that is a defect.
     Diag(VDecl->getLocation(), diag::err_block_extern_cant_init);
     VDecl->setInvalidDecl();
     return;
   }
 
   if (!VDecl->getType()->isDependentType()) {
     // A definition must end up with a complete type, which means it must be
     // complete with the restriction that an array type might be completed by
     // the initializer; note that later code assumes this restriction.
     QualType BaseDeclType = VDecl->getType();
     if (const ArrayType *Array = Context.getAsIncompleteArrayType(BaseDeclType))
       BaseDeclType = Array->getElementType();
     if (RequireCompleteType(VDecl->getLocation(), BaseDeclType,
                             diag::err_typecheck_decl_incomplete_type)) {

clang/test/Parser/cxx1z-decomposition.cpp

 namespace BadSpecifiers {
   typedef int I1[1];
   I1 n;
   struct S { int n; } s;
   void f() {
     // storage-class-specifiers
     static auto &[a] = n; // expected-warning {{declared 'static' is a C++20 extension}}
     thread_local auto &[b] = n; // expected-warning {{declared 'thread_local' is a C++20 extension}}
-    extern auto &[c] = n; // expected-error {{cannot be declared 'extern'}} expected-error {{cannot have an initializer}}
+    extern auto &[c] = n; // expected-error {{cannot be declared 'extern'}} expected-error {{declaration of block scope identifier with linkage cannot have an initializer}}
     struct S {
       mutable auto &[d] = n; // expected-error {{not permitted in this context}}
 
       // function-specifiers
       virtual auto &[e] = n; // expected-error {{not permitted in this context}}
       explicit auto &[f] = n; // expected-error {{not permitted in this context}}
 
       // misc decl-specifiers

clang/test/Sema/array-init.c

void func(void) {
   };
 
   struct threeElements {
     int a,b,c;
   } z = { 1 };
 
   struct threeElements *p = 7; // expected-error{{incompatible integer to pointer conversion initializing 'struct threeElements *' with an expression of type 'int'}}
 
-  extern int blockScopeExtern[3] = { 1, 3, 5 }; // expected-error{{'extern' variable cannot have an initializer}}
+  extern int blockScopeExtern[3] = { 1, 3, 5 }; // expected-error{{declaration of block scope identifier with linkage cannot have an initializer}}
 
   static long x2[3] = { 1.0,
                         "abc", // expected-error{{incompatible pointer to integer conversion initializing 'long' with an expression of type 'char[4]'}}
                          5.8 }; // expected-warning {{implicit conversion from 'double' to 'long' changes value from 5.8 to 5}}
 }
 
 void test(void) {
   int y1[3] = {

clang/test/Sema/err-decl-block-extern-no-init.c

+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
+static int x;
+
+void foo(void)
+{
+    extern int x = 1; // expected-error {{declaration of block scope identifier with linkage cannot have an initializer}}
+}
+
+int y;
+
+void bar(void)
+{
+    extern int y = 1; // expected-error {{declaration of block scope identifier with linkage cannot have an initializer}}
+
+}

clang/test/Sema/private-extern.c

 struct s0;
 void f8(void) {
   extern struct s0 g14;
   __private_extern__ struct s0 g14;
 }
 struct s0 { int x; };
 
 void f9(void) {
-  extern int g15 = 0; // expected-error{{'extern' variable cannot have an initializer}}
+  extern int g15 = 0; // expected-error{{declaration of block scope identifier with linkage cannot have an initializer}}
   // FIXME: linkage specifier in warning.
-  __private_extern__ int g16 = 0; // expected-error{{'extern' variable cannot have an initializer}}
+  __private_extern__ int g16 = 0; // expected-error{{declaration of block scope identifier with linkage cannot have an initializer}}
 }
 
 extern int g17;
 int g17 = 0;
 
 extern int g18 = 0; // expected-warning{{'extern' variable has an initializer}}
 
 __private_extern__ int g19;
 int g19 = 0;
 
 __private_extern__ int g20 = 0;