aaron.ballman added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5904-5905
def err_block_extern_cant_init : Error<
- "'extern' variable cannot have an initializer">;
+ "declaration of block scope identifier with %select{external|internal}0 "
+ "linkage shall have no initializer">;
def warn_extern_init : Warning<"'extern' variable has an initializer">,
----------------
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.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:12774-12784
+ // C99 6.7.8p5. C++ has no such restriction, but that is a defect.
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);
+ unsigned LinkageKind = /*external*/ 0;
+ // C2x 6.7.10p6.
+ if (VDecl->getFormalLinkage() == InternalLinkage)
+ LinkageKind = /*internal*/ 1;
+
----------------
================
Comment at: clang/lib/Sema/SemaDecl.cpp:12776-12778
+ unsigned LinkageKind = /*external*/ 0;
+ // C2x 6.7.10p6.
+ if (VDecl->getFormalLinkage() == InternalLinkage)
----------------
================
Comment at: clang/test/Sema/err-decl-block-extern-no-init.c:6
+{
+ extern int x = 1; // expected-error {{declaration of block scope
identifier with internal linkage shall have no initializer}}
+}
----------------
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.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133088/new/
https://reviews.llvm.org/D133088
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
