[clang] [Clang] Diagnose UB and emit error when identifier has both internal and external linkage (PR #192116)

[Aaron Ballman via cfe-commits]

AaronBallman wrote:

> TL;DR: CWG426 was superseded by [P1787R6](https://wg21.link/p1787r6), @AaronBallman apparently got confused by an AST dump, `extern` declarations can have internal linkage since C89, and comments in `linkage-internal-extern.cpp` are entirely wrong.

Great catch, thank you for this! Let's revert the PR so that @flash1729 can fix things up and we can re-land in a better state.

> > (look at the AST dump for the final declarations of i and f and see we give them external linkage while the standard now says they should have internal linkage).
> 
> If you interpreted `extern` at the end as an indication of linkage, this is how AST dump looks like with `getFormalLinkage()`:

Ugh, yeah, I got caught by that. It turns out that even a JSON dump is a bit confusing, it only lists the storage class but not the formal linkage. Tricky!

> ```
> |-FunctionDecl 0x55657e43d9d0 <test.cxx:1:1, col:15> col:13 f 'void ()' static internal-linkage
> |-LinkageSpecDecl 0x55657e43dae0 <line:2:1, col:19> col:8 C
> | `-FunctionDecl 0x55657e43db50 <col:12, col:19> col:17 h 'void ()' external-linkage
> |-VarDecl 0x55657e43dc10 <line:3:1, col:16> col:12 i 'int' static cinit internal-linkage
> | `-IntegerLiteral 0x55657e43dc78 <col:16> 'int' 0
> `-FunctionDecl 0x55657e43dd00 <line:4:1, line:13:1> line:4:6 q 'void ()' external-linkage
>   `-CompoundStmt 0x55657e43e2c8 <col:10, line:13:1>
>     |-DeclStmt 0x55657e43de80 <line:5:3, col:18>
>     | `-FunctionDecl 0x55657e43ddc8 parent 0x55657e3eea58 prev 0x55657e43d9d0 <col:3, col:17> col:15 f 'void ()' extern internal-linkage
>     |-DeclStmt 0x55657e43df70 <line:6:3, col:18>
>     | `-FunctionDecl 0x55657e43deb8 parent 0x55657e3eea58 <col:3, col:17> col:15 g 'void ()' extern external-linkage
>     |-DeclStmt 0x55657e43e060 <line:7:3, col:18>
>     | `-FunctionDecl 0x55657e43dfa8 parent 0x55657e3eea58 prev 0x55657e43db50 <col:3, col:17> col:15 h 'void ()' extern external-linkage
>     |-DeclStmt 0x55657e43e0f8 <line:8:3, col:8>
>     | `-VarDecl 0x55657e43e090 <col:3, col:7> col:7 i 'int' no-linkage
>     `-CompoundStmt 0x55657e43e2a8 <line:9:3, line:12:3>
>       |-DeclStmt 0x55657e43e1e8 <line:10:5, col:20>
>       | `-FunctionDecl 0x55657e43e130 parent 0x55657e3eea58 prev 0x55657e43ddc8 <col:5, col:19> col:17 f 'void ()' extern internal-linkage
>       `-DeclStmt 0x55657e43e290 <line:11:5, col:17>
>         `-VarDecl 0x55657e43e218 parent 0x55657e3eea58 prev 0x55657e43dc10 <col:5, col:16> col:16 i 'int' extern internal-linkage
> ```
> 
> As you can see, the last two declarations (of `i` and `f`, respectively) have internal linkage. This was observed in our default GNU++17 mode.
> 
> When I checked a similar C example taken from N3410, the block scope extern had external linkage, as the C wording says.
> 
> > This is https://cplusplus.github.io/CWG/issues/426.html
> 
> This Core issue and its resolution were superseded by [P1787R6](https://wg21.link/p1787r6) "Declarations and where to find them", adopted into C++23 (not as a DR, but we handle the example in [[basic.link]/6](https://eel.is/c++draft/basic.link#6) the same way in C++98 mode as in GNU++17 mode).
> ## `extern` declarations with internal linkage in C
> 
> > Amazingly, you can declare the variable as extern in C++ and it will have internal linkage (wow)
> 
> C89 3.1.2.2 Linkage of identifiers:
> 
> > If the declaration of an identifier for an object or a function contains the storage-class specifier `extern`, the identifier has the same linkage as any visible declaration of the identifier with file scope. If there is no visible declaration with file scope, the identifier has external linkage.
> 
> ```c
> static int k = 0; // internal linkage
> void f() {
>   extern int k; // inherits internal linkage from prior visible declaration per C89 wording
> }
> ```
> 
> ```
> |-VarDecl 0x55996df164b0 <test.c:1:1, col:16> col:12 k 'int' static cinit internal-linkage
> | `-IntegerLiteral 0x55996df16560 <col:16> 'int' 0
> `-FunctionDecl 0x55996df16620 <line:2:1, line:4:1> line:2:6 f 'void ()' external-linkage
>   `-CompoundStmt 0x55996df16770 <col:10, line:4:1>
>     `-DeclStmt 0x55996df16758 <line:3:3, col:15>
>       `-VarDecl 0x55996df166e0 parent 0x55996deae9d8 prev 0x55996df164b0 <col:3, col:14> col:14 k 'int' extern internal-linkage
> ```

Ah, now that I see the formal linkage dumped, yup. The wording in C2y is different from C89 but is functionally equivalent.



https://github.com/llvm/llvm-project/pull/192116