Skip to content
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

[clang][bytecode] Fix defining extern variables #108940

Merged
merged 1 commit into from
Sep 17, 2024
Merged

Conversation

tbaederr
Copy link
Contributor

At the point of defintion of the variable, a function might already refert to the variable by its index. Replace the index with the new one.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 17, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 17, 2024

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

At the point of defintion of the variable, a function might already refert to the variable by its index. Replace the index with the new one.


Full diff: https://github.com/llvm/llvm-project/pull/108940.diff

2 Files Affected:

  • (modified) clang/lib/AST/ByteCode/Program.cpp (+10-1)
  • (added) clang/test/AST/ByteCode/extern.cpp (+13)
diff --git a/clang/lib/AST/ByteCode/Program.cpp b/clang/lib/AST/ByteCode/Program.cpp
index bd5860beabaecd..79c645257306e0 100644
--- a/clang/lib/AST/ByteCode/Program.cpp
+++ b/clang/lib/AST/ByteCode/Program.cpp
@@ -204,9 +204,18 @@ std::optional<unsigned> Program::createGlobal(const ValueDecl *VD,
     IsStatic = false;
     IsExtern = true;
   }
+
+  // Register all previous declarations as well. For extern blocks, just replace
+  // the index with the new variable.
   if (auto Idx = createGlobal(VD, VD->getType(), IsStatic, IsExtern, Init)) {
-    for (const Decl *P = VD; P; P = P->getPreviousDecl())
+    for (const Decl *P = VD; P; P = P->getPreviousDecl()) {
+      if (P != VD) {
+        unsigned PIdx = GlobalIndices[P];
+        if (Globals[PIdx]->block()->isExtern())
+          Globals[PIdx] = Globals[*Idx];
+      }
       GlobalIndices[P] = *Idx;
+    }
     return *Idx;
   }
   return std::nullopt;
diff --git a/clang/test/AST/ByteCode/extern.cpp b/clang/test/AST/ByteCode/extern.cpp
new file mode 100644
index 00000000000000..a616269911a7ec
--- /dev/null
+++ b/clang/test/AST/ByteCode/extern.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify=both,expected %s
+// RUN: %clang_cc1 -verify=both,ref %s
+
+
+// both-no-diagnostics
+
+extern const int E;
+constexpr int getE() {
+  return E;
+}
+const int E = 10;
+static_assert(getE() == 10);
+

At the point of defintion of the variable, a function might already
refert to the variable by its index. Replace the index with the new one.
@tbaederr tbaederr changed the title [clang][bytecode] Fix definining extern variables [clang][bytecode] Fix defining extern variables Sep 17, 2024
@tbaederr tbaederr merged commit 8e2dbab into llvm:main Sep 17, 2024
8 checks passed
tmsri pushed a commit to tmsri/llvm-project that referenced this pull request Sep 19, 2024
At the point of defintion of the variable, a function might already
refert to the variable by its index. Replace the index with the new one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants