Skip to content

Commit

Permalink
[clang-repl] Emit const variables only once (#65257)
Browse files Browse the repository at this point in the history
Disable internal linkage for const variables if IncrementalExtensions
are enabled. Otherwise the variables are emitted multiple times, with
multiple constructions at unique memory locations, during every PTU.
  • Loading branch information
hahnjo committed Oct 3, 2023
1 parent f89b7a9 commit 05137ec
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 0 deletions.
10 changes: 10 additions & 0 deletions clang/lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11768,6 +11768,16 @@ GVALinkage ASTContext::GetGVALinkageForFunction(const FunctionDecl *FD) const {

static GVALinkage basicGVALinkageForVariable(const ASTContext &Context,
const VarDecl *VD) {
// As an extension for interactive REPLs, make sure constant variables are
// only emitted once instead of LinkageComputer::getLVForNamespaceScopeDecl
// marking them as internal.
if (Context.getLangOpts().CPlusPlus &&
Context.getLangOpts().IncrementalExtensions &&
VD->getType().isConstQualified() &&
!VD->getType().isVolatileQualified() && !VD->isInline() &&
!isa<VarTemplateSpecializationDecl>(VD) && !VD->getDescribedVarTemplate())
return GVA_DiscardableODR;

if (!VD->isExternallyVisible())
return GVA_Internal;

Expand Down
29 changes: 29 additions & 0 deletions clang/test/Interpreter/const.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// UNSUPPORTED: system-aix
// RUN: cat %s | clang-repl | FileCheck %s
// RUN: cat %s | clang-repl -Xcc -O2 | FileCheck %s

extern "C" int printf(const char*, ...);

struct A { int val; A(int v); ~A(); void f() const; };
A::A(int v) : val(v) { printf("A(%d), this = %p\n", val, this); }
A::~A() { printf("~A, this = %p, val = %d\n", this, val); }
void A::f() const { printf("f: this = %p, val = %d\n", this, val); }

const A a(1);
// CHECK: A(1), this = [[THIS:0x[0-9a-f]+]]
// The constructor must only be called once!
// CHECK-NOT: A(1)

a.f();
// CHECK-NEXT: f: this = [[THIS]], val = 1
a.f();
// CHECK-NEXT: f: this = [[THIS]], val = 1

%quit
// There must still be no other constructor!
// CHECK-NOT: A(1)

// At the end, we expect exactly one destructor call
// CHECK: ~A
// CHECK-SAME: this = [[THIS]], val = 1
// CHECK-NOT: ~A

4 comments on commit 05137ec

@RKSimon
Copy link
Collaborator

@RKSimon RKSimon commented on 05137ec Oct 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hahnjo This is causing fails on windows builds - please can you take a look? https://lab.llvm.org/buildbot/#/builders/216/builds/28266

@hahnjo
Copy link
Member Author

@hahnjo hahnjo commented on 05137ec Oct 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'm on it. I need to relax the regular expression to capture the pointer value, will commit shortly

@hahnjo
Copy link
Member Author

@hahnjo hahnjo commented on 05137ec Oct 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

committed b6ee41f

@hahnjo
Copy link
Member Author

@hahnjo hahnjo commented on 05137ec Oct 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still failing, it seems the JIT doesn't deduplicate the variable on Windows. Tracked as #68092, but XFAILed for now in commit 4812eec.

Please sign in to comment.