[clang] [clang][Sema] Always clear UndefinedButUsed (PR #73955)
vgvassilev wrote: In principle we could add this test as an xfail to make sure we have this scenario when unloading support is up to that stage. https://github.com/llvm/llvm-project/pull/73955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Always clear UndefinedButUsed (PR #73955)
https://github.com/hahnjo closed https://github.com/llvm/llvm-project/pull/73955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Always clear UndefinedButUsed (PR #73955)
hahnjo wrote:
I tried to craft a test here, but declaration unloading in `clang-repl` is not
powerful enough (yet) to show a (user-visible) consequence of the problem.
On a high level, the problem should be triggered by:
```
clang-repl> template struct A { void f() { } };
clang-repl> A().f();
clang-repl> %undo
clang-repl> A().f();
```
In principle, the `%undo` should remove the implicit template instantiation of
`f` which is then subsequently re-instantiated. This is currently not
implemented in `clang-repl` (we just re-use the first instantiation, which is
wrong in case there are more lines in between that could influence the
instantiation). With debug statements, I can verify that `f` is in
`UndefinedButUsed`, but `getUndefinedButUsed` filters it out.
The slightly more complex
```
clang-repl> template struct A { void f() { } };
clang-repl> A().f();
clang-repl> %undo
clang-repl> %undo
clang-repl> template struct A { void f() { } };
clang-repl> A().f();
```
ie unloading the entire class doesn't work either for the same reason, we never
actually treat the instantiated member function. (Subsequently this leads to
the entire `clang-repl` crashing after printing `definition with same mangled
name '_ZN1AIiE1fEv' as another definition`...)
https://github.com/llvm/llvm-project/pull/73955
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Always clear UndefinedButUsed (PR #73955)
https://github.com/AaronBallman approved this pull request. I'm okay landing these changes without test coverage, though tests are always preferred. LGTM https://github.com/llvm/llvm-project/pull/73955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Always clear UndefinedButUsed (PR #73955)
vgvassilev wrote: I think I understand. @AaronBallman from what concerns me that change seems fine it'd be hard to add a test for it right now. Do you have any concerns? https://github.com/llvm/llvm-project/pull/73955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Always clear UndefinedButUsed (PR #73955)
hahnjo wrote: I will try, but observing the consequences of this depends on unloading: Basically it happens if a declaration in `UndefinedButUsed` thas was previously defined is unloaded, which makes it undefined. For now, it's possible that for the upstream cases it's only an optimization because the data structure doesn't grow as much. https://github.com/llvm/llvm-project/pull/73955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Always clear UndefinedButUsed (PR #73955)
vgvassilev wrote: Can you add a test? https://github.com/llvm/llvm-project/pull/73955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Always clear UndefinedButUsed (PR #73955)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Jonas Hahnfeld (hahnjo)
Changes
Before, it was only cleared if there were undefined entities. This is important
for Clang's incremental parsing as used by `clang-repl` that might receive
multiple calls to `Sema.ActOnEndOfTranslationUnit`.
---
Full diff: https://github.com/llvm/llvm-project/pull/73955.diff
1 Files Affected:
- (modified) clang/lib/Sema/Sema.cpp (+1-2)
``diff
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 9771aaa2f3b0371..d08f8cd56b39bde 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -870,6 +870,7 @@ static void checkUndefinedButUsed(Sema ) {
// Collect all the still-undefined entities with internal linkage.
SmallVector, 16> Undefined;
S.getUndefinedButUsed(Undefined);
+ S.UndefinedButUsed.clear();
if (Undefined.empty()) return;
for (const auto : Undefined) {
@@ -923,8 +924,6 @@ static void checkUndefinedButUsed(Sema ) {
if (UseLoc.isValid())
S.Diag(UseLoc, diag::note_used_here);
}
-
- S.UndefinedButUsed.clear();
}
void Sema::LoadExternalWeakUndeclaredIdentifiers() {
``
https://github.com/llvm/llvm-project/pull/73955
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Always clear UndefinedButUsed (PR #73955)
https://github.com/hahnjo created
https://github.com/llvm/llvm-project/pull/73955
Before, it was only cleared if there were undefined entities. This is important
for Clang's incremental parsing as used by `clang-repl` that might receive
multiple calls to `Sema.ActOnEndOfTranslationUnit`.
>From 9dd0362e1dcffa5475d9f959ce9bfc6a7e83083b Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld
Date: Thu, 30 Nov 2023 16:51:23 +0100
Subject: [PATCH] [clang][Sema] Always clear UndefinedButUsed
Before, it was only cleared if there were undefined entities. This
is important for Clang's incremental parsing as used by clang-repl
that might receive multiple calls to Sema.ActOnEndOfTranslationUnit.
---
clang/lib/Sema/Sema.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 9771aaa2f3b0371..d08f8cd56b39bde 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -870,6 +870,7 @@ static void checkUndefinedButUsed(Sema ) {
// Collect all the still-undefined entities with internal linkage.
SmallVector, 16> Undefined;
S.getUndefinedButUsed(Undefined);
+ S.UndefinedButUsed.clear();
if (Undefined.empty()) return;
for (const auto : Undefined) {
@@ -923,8 +924,6 @@ static void checkUndefinedButUsed(Sema ) {
if (UseLoc.isValid())
S.Diag(UseLoc, diag::note_used_here);
}
-
- S.UndefinedButUsed.clear();
}
void Sema::LoadExternalWeakUndeclaredIdentifiers() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
