hokein wrote:
> I want to somehow record this breakage in the form of a test for our future
> selves when decide to revisit this workaround-looking code.
This is the processed file
https://gist.github.com/hokein/e4a5881329c3956494afa2de7d350476.
> I am a bit overwhelmed right now, are you
vgvassilev wrote:
> > Out of curiosity, in what context you use -fincremental-extensions?
>
> The code snippet I provided is extracted from our internal test. We have an
> internal clang-tool (with the `incremental-extensions` on) to generate headers
I am a bit overwhelmed right now, are you
hokein wrote:
> Out of curiosity, in what context you use -fincremental-extensions?
The code snippet I provided is extracted from our internal test. We have an
internal clang-tool (with the `incremental-extensions` on) to generate headers
https://github.com/llvm/llvm-project/pull/89804
vgvassilev wrote:
> Thanks for the prompt response. I think limiting it to C-only will fix the
> issue (note that there is no `C` in `LangOpts`, you may want to use
> `!getLangOpts().CPlusPlus` to exclude C++).
Makes sense. If that works for you, we can check this in. I want to somehow
hokein wrote:
Thanks for the prompt response. I think limiting it to C-only will fix the
issue (note that there is no `C` in `LangOpts`, you may want to use
`!getLangOpts().CPlusPlus` to exclude C++).
https://github.com/llvm/llvm-project/pull/89804
vgvassilev wrote:
I realize I do not entirely understand the role of the IdResolver chain in c++.
Perhaps we are better of changing this line to:
```cpp
if (!PP.isIncrementalProcessingEnabled() || getLangOpts().ObjC)
IdResolver.RemoveDecl(D);
```
to
```
if
vgvassilev wrote:
@hokein, ah, that's annoying. Can you provide the entire proprocessed file that
does not work? I'd like to bisect and debug.
https://github.com/llvm/llvm-project/pull/89804
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
hokein wrote:
@vgvassilev
The reland d999ce0302f06d250f6d496b56a5a5f2dc331e61 makes the clang reject the
valid code now:
```
$ cat /tmp/t33.cpp
#include
#include
int main() {
}
vgvassilev wrote:
@jasonmolenda, I struggled quite a bit in reproducing the test failures and
everything but I have a fix. Thanks a lot for your time and I hope the fix
works!
https://github.com/llvm/llvm-project/pull/89804
___
cfe-commits mailing
jasonmolenda wrote:
I set aside my current work, updated to current main sources, cherry-picked
253c28fa829cee0104c2fc59ed1a958980b5138c, built it on my macOS system, did
`bin/lldb-dotest -p TestImportBuiltinFileID.py` and it crashes like before.
I'm not sure what you're asking help with, I
vgvassilev wrote:
@mysterymath, @jasonmolenda, ping -- I am still stuck in reproducing this issue.
https://github.com/llvm/llvm-project/pull/89804
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
vgvassilev wrote:
> Just found out this keeps popping up from my notification, and I would like
> give my 2 cents, maybe can provide some insights:
>
> Looking at the backtrace of the failing tests, I noticed we ran into
> `clang::Parser::ParseTopLevelStmtDecl()` which makes me very confused.
vgvassilev wrote:
> fwiw if you have access to a mac, I don't think it's hard to build
> lldb+clang? I have swig, cmake, and ninja installed on my mac via homebrew.
>
> ```
> lldb/scripts/macos-setup-codesign.sh
>
> mkdir build
> cd build
> cmake ../llvm -G Ninja
junaire wrote:
Just found out this keeps popping up from my notification, and I would like
give my 2 cents, maybe can provide some insights:
Looking at the backtrace of the failing tests, I noticed we ran into
`clang::Parser::ParseTopLevelStmtDecl()` which makes me very confused. Do we
jasonmolenda wrote:
fwiw if you have access to a mac, I don't think it's hard to build lldb+clang?
I have swig, cmake, and ninja installed on my mac via homebrew.
```
lldb/scripts/macos-setup-codesign.sh
mkdir build
cd build
cmake ../llvm -G Ninja -DLLDB_ENABLE_SWIFT_SUPPORT=0
mysterymath wrote:
This still exhibits the same issue:
https://luci-milo.appspot.com/raw/build/logs.chromium.org/fuchsia/led/dthorn_google.com/0d77d2b0963e18ad11cb83db0680c2382ed7f8df95cf733f42fcdbd1dda8a7ec/+/build.proto
vgvassilev wrote:
Ok, maybe I can you test this patch for a temporary fix until we figure out
what's going wrong.
```diff
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 7f6921ea22be..d771a060f305 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++
mysterymath wrote:
I did have a means to reproduce this locally, but it's been a long time since
I've used it. I should be able to recreate the setup. I'll get back to you with
the details when I can; this whole afternoon is booked through pretty solid
with meetings for me I'm afraid.
vgvassilev wrote:
> I was able to verify that the revert fixed the tests at least.
I am pretty sure that the failure ha to do with the incremental processing
if-stmt in SemaDecl. I can provide a fix that does not trigger this for lldb
but that would be a workaround. I suspect that change
vgvassilev wrote:
> > @jasonmolenda, I am stuck. I could not find how the bot configures llvm and
> > lldb. I built lldb and ran `make check-lldb` but did not fail in the same
> > way. Can you provide a recipe to reproduce the failure?
>
> For some reason it only fails on our Mac LLDB
mysterymath wrote:
I was able to verify that the revert fixed the tests at least.
https://github.com/llvm/llvm-project/pull/89804
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mysterymath wrote:
> @jasonmolenda, I am stuck. I could not find how the bot configures llvm and
> lldb. I built lldb and ran `make check-lldb` but did not fail in the same
> way. Can you provide a recipe to reproduce the failure?
For some reason it only fails on our Mac LLDB builders, and
vgvassilev wrote:
@jasonmolenda, I am stuck. I could not find how the bot configures llvm and
lldb. I built lldb and ran `make check-lldb` but did not fail in the same way.
Can you provide a recipe to reproduce the failure?
https://github.com/llvm/llvm-project/pull/89804
vgvassilev wrote:
> I reverted in
>
> ```
> commit dfdf1c5fe45a82b9c578306f3d7627fd251d63f8
> Author: Jason Molenda
> Date: Tue May 21 18:00:11 2024 -0700
>
> Revert "[clang-repl] Extend the C support. (#89804)"
>
> This reverts commit 253c28fa829cee0104c2fc59ed1a958980b5138c.
jasonmolenda wrote:
I reverted in
```
commit dfdf1c5fe45a82b9c578306f3d7627fd251d63f8
Author: Jason Molenda
Date: Tue May 21 18:00:11 2024 -0700
Revert "[clang-repl] Extend the C support. (#89804)"
This reverts commit 253c28fa829cee0104c2fc59ed1a958980b5138c.
```
to unblock
mysterymath wrote:
We've started seeing crashes in Fuchsia's Mac tooclhain's LLDB test suite:
https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-mac-x64/b8747305520511321169/overview
This is the only PR in the blamelist that looks plausible, but there isn't a
clear
https://github.com/vgvassilev closed
https://github.com/llvm/llvm-project/pull/89804
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
vgvassilev wrote:
The failures on windows are due to "out of heap" error. Let's land this to test
on the "real" bots.
https://github.com/llvm/llvm-project/pull/89804
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
@@ -407,6 +406,16 @@ void IncrementalParser::CleanUpPTU(PartialTranslationUnit
) {
}
}
}
+
+ // FIXME: We should de-allocate MostRecentTU
+ for (Decl *D : MostRecentTU->decls()) {
+auto *ND = dyn_cast(D);
+if (!ND)
+ continue;
+// Check if we
https://github.com/AaronBallman approved this pull request.
LGTM with a possible improvement in the code (take it or leave it, your call).
https://github.com/llvm/llvm-project/pull/89804
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
@@ -407,6 +406,16 @@ void IncrementalParser::CleanUpPTU(PartialTranslationUnit
) {
}
}
}
+
+ // FIXME: We should de-allocate MostRecentTU
+ for (Decl *D : MostRecentTU->decls()) {
+auto *ND = dyn_cast(D);
+if (!ND)
+ continue;
+// Check if we
https://github.com/AaronBallman edited
https://github.com/llvm/llvm-project/pull/89804
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/vgvassilev updated
https://github.com/llvm/llvm-project/pull/89804
>From 0621608c575d9e4f7da5b08db143dbe88a45745a Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Tue, 23 Apr 2024 18:07:06 +
Subject: [PATCH] [clang-repl] Extend the C support.
The IdResolver chain is
vgvassilev wrote:
The bots fail on windows due to `TEST 'Clang ::
CoverageMapping/mcdc-system-headers.cpp' FAILED` which does not seem related to
this PR.
https://github.com/llvm/llvm-project/pull/89804
___
cfe-commits mailing list
https://github.com/vgvassilev updated
https://github.com/llvm/llvm-project/pull/89804
>From 7acf367f2bd36409bcbb7443451ec83188ab589d Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Tue, 23 Apr 2024 18:07:06 +
Subject: [PATCH] [clang-repl] Extend the C support.
The IdResolver chain is
@@ -2282,7 +2282,8 @@ void Sema::ActOnPopScope(SourceLocation Loc, Scope *S) {
// Remove this name from our lexical scope, and warn on it if we haven't
// already.
-IdResolver.RemoveDecl(D);
+if (!PP.isIncrementalProcessingEnabled())
+
https://github.com/vgvassilev updated
https://github.com/llvm/llvm-project/pull/89804
>From 72d655c919ae29f83f1a39db2a63aa5468fb52cc Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Tue, 23 Apr 2024 18:07:06 +
Subject: [PATCH] [clang-repl] Extend the C support.
The IdResolver chain is
@@ -407,6 +406,16 @@ void IncrementalParser::CleanUpPTU(PartialTranslationUnit
) {
}
}
}
+
+ // FIXME: We should de-allocate MostRecentTU
+ for (Decl *D : MostRecentTU->decls()) {
+if (!isa(D))
+ continue;
+// Check if we need to clean up the
@@ -407,6 +406,16 @@ void IncrementalParser::CleanUpPTU(PartialTranslationUnit
) {
}
}
}
+
+ // FIXME: We should de-allocate MostRecentTU
+ for (Decl *D : MostRecentTU->decls()) {
+if (!isa(D))
+ continue;
+// Check if we need to clean up the
@@ -407,6 +406,16 @@ void IncrementalParser::CleanUpPTU(PartialTranslationUnit
) {
}
}
}
+
+ // FIXME: We should de-allocate MostRecentTU
+ for (Decl *D : MostRecentTU->decls()) {
+if (!isa(D))
+ continue;
+// Check if we need to clean up the
@@ -407,6 +406,16 @@ void IncrementalParser::CleanUpPTU(PartialTranslationUnit
) {
}
}
}
+
+ // FIXME: We should de-allocate MostRecentTU
+ for (Decl *D : MostRecentTU->decls()) {
+if (!isa(D))
+ continue;
+// Check if we need to clean up the
@@ -2282,7 +2282,8 @@ void Sema::ActOnPopScope(SourceLocation Loc, Scope *S) {
// Remove this name from our lexical scope, and warn on it if we haven't
// already.
-IdResolver.RemoveDecl(D);
+if (!PP.isIncrementalProcessingEnabled())
+
@@ -407,6 +406,16 @@ void IncrementalParser::CleanUpPTU(PartialTranslationUnit
) {
}
}
}
+
+ // FIXME: We should de-allocate MostRecentTU
+ for (Decl *D : MostRecentTU->decls()) {
+if (!isa(D))
+ continue;
+// Check if we need to clean up the
@@ -2282,7 +2282,8 @@ void Sema::ActOnPopScope(SourceLocation Loc, Scope *S) {
// Remove this name from our lexical scope, and warn on it if we haven't
// already.
-IdResolver.RemoveDecl(D);
+if (!PP.isIncrementalProcessingEnabled())
+
@@ -407,6 +406,16 @@ void IncrementalParser::CleanUpPTU(PartialTranslationUnit
) {
}
}
}
+
+ // FIXME: We should de-allocate MostRecentTU
+ for (Decl *D : MostRecentTU->decls()) {
+if (!isa(D))
+ continue;
+// Check if we need to clean up the
@@ -407,6 +406,16 @@ void IncrementalParser::CleanUpPTU(PartialTranslationUnit
) {
}
}
}
+
+ // FIXME: We should de-allocate MostRecentTU
+ for (Decl *D : MostRecentTU->decls()) {
+if (!isa(D))
+ continue;
+// Check if we need to clean up the
@@ -2282,7 +2282,8 @@ void Sema::ActOnPopScope(SourceLocation Loc, Scope *S) {
// Remove this name from our lexical scope, and warn on it if we haven't
// already.
-IdResolver.RemoveDecl(D);
+if (!PP.isIncrementalProcessingEnabled())
+
@@ -407,6 +406,16 @@ void IncrementalParser::CleanUpPTU(PartialTranslationUnit
) {
}
}
}
+
+ // FIXME: We should de-allocate MostRecentTU
+ for (Decl *D : MostRecentTU->decls()) {
+if (!isa(D))
+ continue;
+// Check if we need to clean up the
@@ -407,6 +406,16 @@ void IncrementalParser::CleanUpPTU(PartialTranslationUnit
) {
}
}
}
+
+ // FIXME: We should de-allocate MostRecentTU
+ for (Decl *D : MostRecentTU->decls()) {
+if (!isa(D))
+ continue;
+// Check if we need to clean up the
vgvassilev wrote:
@AaronBallman, can you take a look at that patch, hopefully to move forward as
seems the other reviewers are busy.
https://github.com/llvm/llvm-project/pull/89804
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
vgvassilev wrote:
The pre-merge windows test failure in `Clang :: Driver/amdgpu-toolchain.c`
seems unrelated to this PR.
https://github.com/llvm/llvm-project/pull/89804
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/vgvassilev updated
https://github.com/llvm/llvm-project/pull/89804
>From 50fa9c91bee980a114ebab2b3301a1e378dd2b81 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Tue, 23 Apr 2024 18:07:06 +
Subject: [PATCH] [clang-repl] Extend the C support.
The IdResolver chain is
https://github.com/vgvassilev updated
https://github.com/llvm/llvm-project/pull/89804
>From 8317ce33d07d0986e314de0b39aa977f784e0619 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev
Date: Tue, 23 Apr 2024 18:07:06 +
Subject: [PATCH] [clang-repl] Extend the C support.
The IdResolver chain is
@@ -0,0 +1,21 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+
+// RUN: cat %s | clang-repl -Xcc -xc -Xcc -Xclang -Xcc -verify | FileCheck %s
+// RUN: cat %s | clang-repl -Xcc -xc -Xcc -O2 -Xcc -Xclang -Xcc -verify|
FileCheck %s
+int printf(const char *, ...);
@@ -407,6 +406,16 @@ void IncrementalParser::CleanUpPTU(PartialTranslationUnit
) {
}
}
}
+
+ // FIXME: We should de-allocate MostRecentTU
+ for (Decl *D : MostRecentTU->decls()) {
+if (!isa(D))
+ continue;
+// Check if we need to clean up the
@@ -407,6 +406,16 @@ void IncrementalParser::CleanUpPTU(PartialTranslationUnit
) {
}
}
}
+
+ // FIXME: We should de-allocate MostRecentTU
+ for (Decl *D : MostRecentTU->decls()) {
+if (!isa(D))
+ continue;
+// Check if we need to clean up the
https://github.com/weliveindetail edited
https://github.com/llvm/llvm-project/pull/89804
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/weliveindetail commented:
I am not an expert on the behavior of IdResolver, but this patch works for me.
https://github.com/llvm/llvm-project/pull/89804
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
@@ -0,0 +1,21 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+
+// RUN: cat %s | clang-repl -Xcc -xc -Xcc -Xclang -Xcc -verify | FileCheck %s
+// RUN: cat %s | clang-repl -Xcc -xc -Xcc -O2 -Xcc -Xclang -Xcc -verify|
FileCheck %s
+int printf(const char *, ...);
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Vassil Vassilev (vgvassilev)
Changes
The IdResolver chain is the main way for C to implement lookup rules. Every new
partial translation unit caused clang to exit the top-most scope which in turn
cleaned up the IdResolver chain. That was
https://github.com/vgvassilev created
https://github.com/llvm/llvm-project/pull/89804
The IdResolver chain is the main way for C to implement lookup rules. Every new
partial translation unit caused clang to exit the top-most scope which in turn
cleaned up the IdResolver chain. That was not an
61 matches
Mail list logo
