-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[C2y] Allow static local variables in inline functions with external … #167086
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…linkage (N3622) This allows static local variables to be declared in inline functions with external linkage, a constraint that was removed in WG14 N3622. Such declarations are now allowed in C2y mode, and accepted as an extension in earlier language models. The code changes carried out were heavily inspired by commit 8e60adc, which implemented making use of static variables or functions within extern inline functions as part of the same paper.
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-clang Author: Pranjal Prajapati (Pranjal095) Changes…linkage (N3622) This allows static local variables to be declared in inline functions with external linkage, a constraint that was removed in WG14 N3622. Such declarations are now allowed in C2y mode, and accepted as an extension in earlier language models. The code changes carried out were heavily inspired by commit 8e60adc, which implemented making use of static variables or functions within extern inline functions as part of the same paper. Resolves #166623 Full diff: https://github.com/llvm/llvm-project/pull/167086.diff 5 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 1e0321de3f4b6..2c2e249282246 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1401,7 +1401,7 @@ def C23 : DiagGroup<"c23-extensions", [VariadicMacroArgumentsOmitted]>;
def : DiagGroup<"c2x-extensions", [C23]>;
// A warning group for warnings about using C2y features as extensions.
-def C2y : DiagGroup<"c2y-extensions", [StaticInInline]>;
+def C2y : DiagGroup<"c2y-extensions", [StaticInInline, StaticLocalInInline]>;
// Previously supported warning group which is no longer pertinent as binary
// literals are a C++14 and C23 extension now instead of a GNU extension.
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3e864475f22a1..28fbefe4c6663 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6339,9 +6339,18 @@ def warn_c2y_compat_internal_in_extern_inline : Warning<
"using static %select{function|variable}0 %1 in an inline function with "
"external linkage is incompatible with standards before C2y">,
InGroup<CPre2yCompat>, DefaultIgnore;
-def warn_static_local_in_extern_inline : Warning<
- "non-constant static local variable in inline function may be different "
- "in different files">, InGroup<StaticLocalInInline>;
+def ext_static_local_in_extern_inline
+ : ExtWarn<"non-constant static local variable in an inline function with "
+ "external linkage is a C2y extension">,
+ InGroup<StaticLocalInInline>;
+def ext_static_local_in_extern_inline_quiet
+ : Extension<ext_static_local_in_extern_inline.Summary>,
+ InGroup<StaticLocalInInline>;
+def warn_c2y_compat_static_local_in_extern_inline
+ : Warning<"non-constant static local variable in an inline function with "
+ "external linkage is incompatible with standards before C2y">,
+ InGroup<CPre2yCompat>,
+ DefaultIgnore;
def note_convert_inline_to_static : Note<
"use 'static' to give inline function %0 internal linkage">;
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 086dd8ba1c670..934d17d57f9dd 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -8111,6 +8111,11 @@ NamedDecl *Sema::ActOnVariableDeclarator(
// An inline definition of a function with external linkage shall
// not contain a definition of a modifiable object with static or
// thread storage duration...
+ //
+ // WG14 N3622 which removed the constraint entirely in C2y. It is left
+ // enabled in earlier language modes because this is a constraint in those
+ // language modes. But in C2y mode, we still want to issue the "incompatible
+ // with previous standards" diagnostic, too.
// We only apply this when the function is required to be defined
// elsewhere, i.e. when the function is not 'extern inline'. Note
// that a local variable with thread storage duration still has to
@@ -8120,8 +8125,15 @@ NamedDecl *Sema::ActOnVariableDeclarator(
!NewVD->getType().isConstQualified()) {
FunctionDecl *CurFD = getCurFunctionDecl();
if (CurFD && isFunctionDefinitionDiscarded(*this, CurFD)) {
- Diag(D.getDeclSpec().getStorageClassSpecLoc(),
- diag::warn_static_local_in_extern_inline);
+ unsigned DiagID;
+ if (getLangOpts().C2y)
+ DiagID = diag::warn_c2y_compat_static_local_in_extern_inline;
+ else if (getSourceManager().isInMainFile(D.getBeginLoc()))
+ DiagID = diag::ext_static_local_in_extern_inline_quiet;
+ else
+ DiagID = diag::ext_static_local_in_extern_inline;
+
+ Diag(D.getDeclSpec().getStorageClassSpecLoc(), DiagID);
MaybeSuggestAddingStaticToDecl(CurFD);
}
}
diff --git a/clang/test/C/C2y/n3622_1.c b/clang/test/C/C2y/n3622_1.c
new file mode 100644
index 0000000000000..219b412022f4a
--- /dev/null
+++ b/clang/test/C/C2y/n3622_1.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -verify=good -pedantic -Wall -std=c2y %s
+// RUN: %clang_cc1 -verify=compat,expected -pedantic -Wall -Wpre-c2y-compat -std=c2y %s
+// RUN: %clang_cc1 -verify=ped,expected -pedantic -Wall -std=c23 %s
+// RUN: %clang_cc1 -verify=ped,expected -pedantic -Wall -std=c17 %s
+// good-no-diagnostics
+
+/* WG14 N3622: Clang 22
+ * Allow static local variables in extern inline functions
+ *
+ * This verifies that a constraint from previous standards is no longer
+ * triggered in C2y mode. The constraint is regarding static local
+ * variables in inline functions with external linkage.
+ */
+
+inline void func1(void) { // expected-note {{use 'static' to give inline function 'func1' internal linkage}}
+ static int x = 0; /* ped-warning {{non-constant static local variable in an inline function with external linkage is a C2y extension}}
+ compat-warning {{non-constant static local variable in an inline function with external linkage is incompatible with standards before C2y}}
+ */
+ (void)x;
+}
+
+inline void func2(void) {
+ static const int x = 0;
+ (void)x;
+}
\ No newline at end of file
diff --git a/clang/test/Sema/inline.c b/clang/test/Sema/inline.c
index 6361db8496c2d..ea65b2cb82997 100644
--- a/clang/test/Sema/inline.c
+++ b/clang/test/Sema/inline.c
@@ -73,9 +73,9 @@ inline int useStaticAgain (void) { // expected-note 2 {{use 'static' to give inl
#pragma clang diagnostic pop
-inline void defineStaticVar(void) { // expected-note {{use 'static' to give inline function 'defineStaticVar' internal linkage}}
+inline void defineStaticVar(void) { // ok (no -pedantic)
static const int x = 0; // ok
- static int y = 0; // expected-warning {{non-constant static local variable in inline function may be different in different files}}
+ static int y = 0; // ok (no -pedantic)
}
extern inline void defineStaticVarInExtern(void) {
|
| // enabled in earlier language modes because this is a constraint in those | ||
| // language modes. But in C2y mode, we still want to issue the "incompatible | ||
| // with previous standards" diagnostic, too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary comment? We should issue "incompatible" diagnostic for all features.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do see your point. However, I wanted to ensure it was as consistent as possible with the linked commit.
| (void)x; | ||
| } | ||
|
|
||
| inline void func2(void) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this test used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first test is to ensure that the new C2y feature works with the specified diagnostics. The second test is to ensure that no false positives have popped up during the process.
AaronBallman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on this!
AaronBallman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've got competing changes, see #166332 for another implementation of this.
Please coordinate with the other author to come up with a single patch.
…linkage (N3622)
This allows static local variables to be declared in inline functions with external linkage, a constraint that was removed in WG14 N3622. Such declarations are now allowed in C2y mode, and accepted as an extension in earlier language models. The code changes carried out were heavily inspired by commit 8e60adc, which implemented making use of static variables or functions within extern inline functions as part of the same paper.
Resolves #166623