[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-10-03 Thread Jonas Hahnfeld via cfe-commits 

hahnjo wrote:

Note that this currently doesn't seem to work on Windows: 
https://github.com/llvm/llvm-project/issues/68092

https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-10-03 Thread Jonas Hahnfeld via cfe-commits 

https://github.com/hahnjo closed https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-10-03 Thread Vassil Vassilev via cfe-commits 

https://github.com/vgvassilev edited 
https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-10-03 Thread Vassil Vassilev via cfe-commits 

https://github.com/vgvassilev approved this pull request.

LGTM. Let's move forward here and rely on a further post-commit review if 
necessary.

https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-10-03 Thread Vassil Vassilev via cfe-commits 


@@ -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*, ...);
+

vgvassilev wrote:

> const A a(1); is a file-scope constant, no?

Yes, missed that.

https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-10-03 Thread Jonas Hahnfeld via cfe-commits 


@@ -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*, ...);
+

hahnjo wrote:

`const A a(1);` is a file-scope constant, no? We don't need it for C because 
the special case in `LinkageComputer::getLVForNamespaceScopeDecl` only applies 
to C++ (my understanding is that `const` variables always have an identity in 
C).

https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-09-28 Thread Vassil Vassilev via cfe-commits 


@@ -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*, ...);
+

vgvassilev wrote:

Can you add a case for a file scope constant? We also should consider enabling 
that for C. 

https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-09-28 Thread Jonas Hahnfeld via cfe-commits 

hahnjo wrote:

ping @vgvassilev 

https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-09-28 Thread Iain Sandoe via cfe-commits 

https://github.com/iains commented:

(still getting used to the revised system)
I have no comments on this patch.


https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-09-18 Thread Jonas Hahnfeld via cfe-commits 

hahnjo wrote:

Ping on the updated patch, which is now also passing downstream testing after I 
realized that we were missing a backport of commit 
e451d552348bc714614d294e32dfbe7ec2cd4005, which explains why some constructors 
were wrongly ordered...

https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-09-06 Thread Jonas Hahnfeld via cfe-commits 

https://github.com/hahnjo review_requested 
https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-09-06 Thread Jonas Hahnfeld via cfe-commits 

hahnjo wrote:

The original patch worked for `clang-repl` but results in strong linkage which 
I found to cause problems with modules downstream in ROOT. Instead the latest 
push moves the special case one level higher to `basicGVALinkageForVariable` 
and returns `GVA_DiscardableODR` which fixes that problem as well.

https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-09-06 Thread Jonas Hahnfeld via cfe-commits 

https://github.com/hahnjo updated 
https://github.com/llvm/llvm-project/pull/65257:

>From 7b52d2ad531286ca3e14c3f05da51c91fd71bd0d Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld 
Date: Wed, 6 Sep 2023 13:11:57 +0200
Subject: [PATCH] [clang-repl] Emit const variables only once

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.
---
 clang/lib/AST/ASTContext.cpp | 10 ++
 clang/test/Interpreter/const.cpp | 29 +
 2 files changed, 39 insertions(+)
 create mode 100644 clang/test/Interpreter/const.cpp

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 4b1d9e86797b77..f7438e9be19ee1 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -11764,6 +11764,16 @@ GVALinkage ASTContext::GetGVALinkageForFunction(const 
FunctionDecl *FD) const {
 
 static GVALinkage basicGVALinkageForVariable(const ASTContext ,
  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(VD) && 
!VD->getDescribedVarTemplate())
+return GVA_DiscardableODR;
+
   if (!VD->isExternallyVisible())
 return GVA_Internal;
 
diff --git a/clang/test/Interpreter/const.cpp b/clang/test/Interpreter/const.cpp
new file mode 100644
index 00..a4b610f1a19d84
--- /dev/null
+++ b/clang/test/Interpreter/const.cpp
@@ -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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-09-05 Thread Aaron Ballman via cfe-commits 

AaronBallman wrote:

> That looks good to me. Can we backport that to the llvm17 release branch? I 
> know at least one downstream project that jumps through hoops to support this.

I'd say we should not backport to 17.0.0; we need to get the final rc out the 
door and this isn't fixing a critical bug or regression. However, it may be 
reasonable to consider for 17.0.1+

https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-09-04 Thread Vassil Vassilev via cfe-commits 

https://github.com/vgvassilev approved this pull request.

That looks good to me. Can we backport that to the llvm17 release branch? I 
know at least one downstream project that jumps through hoops to support this.

https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-09-04 Thread Vassil Vassilev via cfe-commits 

https://github.com/vgvassilev review_requested 
https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-09-04 Thread Jonas Hahnfeld via cfe-commits 

https://github.com/hahnjo review_requested 
https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-09-04 Thread Jonas Hahnfeld via cfe-commits 

https://github.com/hahnjo review_requested 
https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-09-04 Thread Jonas Hahnfeld via cfe-commits 

https://github.com/hahnjo review_requested 
https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-09-04 Thread Jonas Hahnfeld via cfe-commits 

https://github.com/hahnjo created 
https://github.com/llvm/llvm-project/pull/65257:

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.

>From fcea5762fab68459aef3f8e82e98fc52e04e1598 Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld 
Date: Mon, 4 Sep 2023 13:56:25 +0200
Subject: [PATCH] [clang-repl] Emit const variables only once

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.
---
 clang/lib/AST/Decl.cpp   |  1 +
 clang/test/Interpreter/const.cpp | 29 +
 2 files changed, 30 insertions(+)
 create mode 100644 clang/test/Interpreter/const.cpp

diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 60c80f2b075336..e019e701196a38 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -651,6 +651,7 @@ LinkageComputer::getLVForNamespaceScopeDecl(const NamedDecl 
*D,
 // internal linkage
 // (There is no equivalent in C99.)
 if (Context.getLangOpts().CPlusPlus && Var->getType().isConstQualified() &&
+!Context.getLangOpts().IncrementalExtensions &&
 !Var->getType().isVolatileQualified() && !Var->isInline() &&
 !isDeclaredInModuleInterfaceOrPartition(Var) &&
 !isa(Var) &&
diff --git a/clang/test/Interpreter/const.cpp b/clang/test/Interpreter/const.cpp
new file mode 100644
index 00..a4b610f1a19d84
--- /dev/null
+++ b/clang/test/Interpreter/const.cpp
@@ -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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-repl] Emit const variables only once (PR #65257)

2023-09-04 Thread Jonas Hahnfeld via cfe-commits 

https://github.com/hahnjo labeled 
https://github.com/llvm/llvm-project/pull/65257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits