github-actions[bot] wrote:
@michael-jabbour-sonarsource Congratulations on having your first Pull Request
(PR) merged into the LLVM Project!
Your changes will be combined with recent changes from other authors, then
tested by our [build bots](https://lab.llvm.org/buildbot/). If there is a
p
https://github.com/cor3ntin closed
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/cor3ntin approved this pull request.
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
michael-jabbour-sonarsource wrote:
Thank you all for looking into this.
Note that I don't have commit access, and I would appreciate it if someone
merges this if/when it is ready to merge!
https://github.com/llvm/llvm-project/pull/114240
___
cfe-comm
https://github.com/michael-jabbour-sonarsource updated
https://github.com/llvm/llvm-project/pull/114240
>From d5c710e5a16f77b5a44bb842d09674a8b40dbd0d Mon Sep 17 00:00:00 2001
From: Michael Jabbour
Date: Fri, 7 Mar 2025 09:32:54 +0100
Subject: [PATCH] [clang] Fix ASTWriter crash after merging n
vgvassilev wrote:
Please rebase this PR.
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/vgvassilev approved this pull request.
LGTM!
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/michael-jabbour-sonarsource updated
https://github.com/llvm/llvm-project/pull/114240
>From 9eede4531ad27ce1d77df492076d5fbcd237fd3f Mon Sep 17 00:00:00 2001
From: Michael Jabbour
Date: Fri, 7 Mar 2025 09:32:54 +0100
Subject: [PATCH] [clang] Fix ASTWriter crash after merging n
https://github.com/michael-jabbour-sonarsource updated
https://github.com/llvm/llvm-project/pull/114240
>From cc3cf25da67c9f8b9edabb318c6011cad9bd2f58 Mon Sep 17 00:00:00 2001
From: Michael Jabbour
Date: Tue, 29 Oct 2024 11:16:09 +0100
Subject: [PATCH 1/9] [NFC] Factor out RetireNodesFromMerged
https://github.com/michael-jabbour-sonarsource edited
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -2551,18 +2551,7 @@ void Sema::MergeTypedefNameDecl(Scope *S,
TypedefNameDecl *New,
// Make the old tag definition visible.
makeMergedDefinitionVisible(Hidden);
- // If this was an unscoped enumeration, yank all of its enumerators
- // out of the scop
@@ -2551,18 +2551,7 @@ void Sema::MergeTypedefNameDecl(Scope *S,
TypedefNameDecl *New,
// Make the old tag definition visible.
makeMergedDefinitionVisible(Hidden);
- // If this was an unscoped enumeration, yank all of its enumerators
- // out of the scop
https://github.com/ilya-biryukov edited
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -2551,18 +2551,7 @@ void Sema::MergeTypedefNameDecl(Scope *S,
TypedefNameDecl *New,
// Make the old tag definition visible.
makeMergedDefinitionVisible(Hidden);
- // If this was an unscoped enumeration, yank all of its enumerators
- // out of the scop
https://github.com/michael-jabbour-sonarsource updated
https://github.com/llvm/llvm-project/pull/114240
>From cc3cf25da67c9f8b9edabb318c6011cad9bd2f58 Mon Sep 17 00:00:00 2001
From: Michael Jabbour
Date: Tue, 29 Oct 2024 11:16:09 +0100
Subject: [PATCH 1/8] [NFC] Factor out RetireNodesFromMerged
https://github.com/michael-jabbour-sonarsource updated
https://github.com/llvm/llvm-project/pull/114240
>From cc3cf25da67c9f8b9edabb318c6011cad9bd2f58 Mon Sep 17 00:00:00 2001
From: Michael Jabbour
Date: Tue, 29 Oct 2024 11:16:09 +0100
Subject: [PATCH 1/8] [NFC] Factor out RetireNodesFromMerged
https://github.com/michael-jabbour-sonarsource updated
https://github.com/llvm/llvm-project/pull/114240
>From cc3cf25da67c9f8b9edabb318c6011cad9bd2f58 Mon Sep 17 00:00:00 2001
From: Michael Jabbour
Date: Tue, 29 Oct 2024 11:16:09 +0100
Subject: [PATCH 1/8] [NFC] Factor out RetireNodesFromMerged
@@ -2551,18 +2551,7 @@ void Sema::MergeTypedefNameDecl(Scope *S,
TypedefNameDecl *New,
// Make the old tag definition visible.
makeMergedDefinitionVisible(Hidden);
- // If this was an unscoped enumeration, yank all of its enumerators
- // out of the scop
@@ -2551,18 +2551,7 @@ void Sema::MergeTypedefNameDecl(Scope *S,
TypedefNameDecl *New,
// Make the old tag definition visible.
makeMergedDefinitionVisible(Hidden);
- // If this was an unscoped enumeration, yank all of its enumerators
- // out of the scop
@@ -2639,6 +2628,19 @@ void Sema::MergeTypedefNameDecl(Scope *S,
TypedefNameDecl *New,
notePreviousDefinition(Old, New->getLocation());
}
+void Sema::CleanupMergedEnum(Scope *S, Decl *New) {
vgvassilev wrote:
I realize that `Sema::ActOnDuplicateDefinition`
michael-jabbour-sonarsource wrote:
Gentle ping :smile:
@vsapsai, @vgvassilev, I think I have responded to your inquiries and added a
few related questions. Could you have a look?
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailin
https://github.com/michael-jabbour-sonarsource edited
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -2551,18 +2551,7 @@ void Sema::MergeTypedefNameDecl(Scope *S,
TypedefNameDecl *New,
// Make the old tag definition visible.
makeMergedDefinitionVisible(Hidden);
- // If this was an unscoped enumeration, yank all of its enumerators
- // out of the scop
https://github.com/michael-jabbour-sonarsource edited
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -2639,6 +2628,19 @@ void Sema::MergeTypedefNameDecl(Scope *S,
TypedefNameDecl *New,
notePreviousDefinition(Old, New->getLocation());
}
+void Sema::CleanupMergedEnum(Scope *S, Decl *New) {
michael-jabbour-sonarsource wrote:
> Is MergeTypedefNameDecl not
@@ -2639,6 +2628,19 @@ void Sema::MergeTypedefNameDecl(Scope *S,
TypedefNameDecl *New,
notePreviousDefinition(Old, New->getLocation());
}
+void Sema::CleanupMergedEnum(Scope *S, Decl *New) {
vgvassilev wrote:
I feel like this is a property of merging. Can
@@ -2551,18 +2551,7 @@ void Sema::MergeTypedefNameDecl(Scope *S,
TypedefNameDecl *New,
// Make the old tag definition visible.
makeMergedDefinitionVisible(Hidden);
- // If this was an unscoped enumeration, yank all of its enumerators
- // out of the scop
https://github.com/michael-jabbour-sonarsource edited
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/michael-jabbour-sonarsource edited
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -2551,18 +2551,7 @@ void Sema::MergeTypedefNameDecl(Scope *S,
TypedefNameDecl *New,
// Make the old tag definition visible.
makeMergedDefinitionVisible(Hidden);
- // If this was an unscoped enumeration, yank all of its enumerators
- // out of the scop
michael-jabbour-sonarsource wrote:
@vsapsai Thank you for sharing the previous patch. This is really interesting.
> For the record, the previous work I've abandoned is
> https://reviews.llvm.org/D114833 Doesn't seem particularly relevant to this
> change to be honest.
As far as I can see, it
michael-jabbour-sonarsource wrote:
@vsapsai Thanks again for the feedback :pray:
> I've realized that the test doesn't have to be Objective-C, the failure is
> reproducible in C. Curiously, we aren't hitting the assertion in
> C++/Objective-C++ but I don't know why.
I dug a bit into this tod
https://github.com/michael-jabbour-sonarsource updated
https://github.com/llvm/llvm-project/pull/114240
>From cc3cf25da67c9f8b9edabb318c6011cad9bd2f58 Mon Sep 17 00:00:00 2001
From: Michael Jabbour
Date: Tue, 29 Oct 2024 11:16:09 +0100
Subject: [PATCH 1/8] [NFC] Factor out RetireNodesFromMerged
https://github.com/vsapsai commented:
I've realized that the test doesn't have to be Objective-C, the failure is
reproducible in C. Curiously, we aren't hitting the assertion in
C++/Objective-C++ but I don't know why.
I wanted to try defining the enums in different scopes (e.g. in a struct) an
https://github.com/michael-jabbour-sonarsource edited
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/michael-jabbour-sonarsource edited
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
michael-jabbour-sonarsource wrote:
@vsapsai Thank you very much for the input. :pray:
> Can you trigger the crash without -ast-dump-all?
> Can you use in the test %clang_cc1 instead of %clang?
I addressed both of these in f6f0887d22172c9db4f602b7f272630a97a708bd. I added
a separate invocatio
@@ -2551,18 +2551,7 @@ void Sema::MergeTypedefNameDecl(Scope *S,
TypedefNameDecl *New,
// Make the old tag definition visible.
makeMergedDefinitionVisible(Hidden);
- // If this was an unscoped enumeration, yank all of its enumerators
- // out of the scop
@@ -2551,18 +2551,7 @@ void Sema::MergeTypedefNameDecl(Scope *S,
TypedefNameDecl *New,
// Make the old tag definition visible.
makeMergedDefinitionVisible(Hidden);
- // If this was an unscoped enumeration, yank all of its enumerators
- // out of the scop
@@ -4034,6 +4034,12 @@ class Sema final : public SemaBase {
void MergeTypedefNameDecl(Scope *S, TypedefNameDecl *New,
LookupResult &OldDecls);
+ /// RetireNodesFromMergedDecl - We have just merged the decl 'New' by making
+ /// another definitio
@@ -2551,18 +2551,7 @@ void Sema::MergeTypedefNameDecl(Scope *S,
TypedefNameDecl *New,
// Make the old tag definition visible.
makeMergedDefinitionVisible(Hidden);
- // If this was an unscoped enumeration, yank all of its enumerators
- // out of the scop
https://github.com/michael-jabbour-sonarsource updated
https://github.com/llvm/llvm-project/pull/114240
>From cc3cf25da67c9f8b9edabb318c6011cad9bd2f58 Mon Sep 17 00:00:00 2001
From: Michael Jabbour
Date: Tue, 29 Oct 2024 11:16:09 +0100
Subject: [PATCH 1/7] [NFC] Factor out RetireNodesFromMerged
https://github.com/michael-jabbour-sonarsource updated
https://github.com/llvm/llvm-project/pull/114240
>From cc3cf25da67c9f8b9edabb318c6011cad9bd2f58 Mon Sep 17 00:00:00 2001
From: Michael Jabbour
Date: Tue, 29 Oct 2024 11:16:09 +0100
Subject: [PATCH 1/6] [NFC] Factor out RetireNodesFromMerged
vsapsai wrote:
In general, personally I quite like the idea of removing decls from the scope.
Though I think when I've tried to do so [in a different context], there were
some problems. I'll try to find what I was doing and why it wasn't working.
It's not a blocker for your change but it can b
vsapsai wrote:
That crash looks pretty annoying, thanks for looking into this issue and
debugging it.
Can you trigger the crash without `-ast-dump-all`? If there is a way to detect
a faulty behaviour without verifying the internal compiler state, it is more
reliable and less fragile to do it
https://github.com/cor3ntin commented:
This flew under the radar, sorry about that and thanks for your patience
Just a couple questions/comments. Also, can you add an entry in
`clang/docs/ReleaseNotes.rst` ? Thanks
https://github.com/llvm/llvm-project/pull/114240
__
@@ -2551,18 +2551,7 @@ void Sema::MergeTypedefNameDecl(Scope *S,
TypedefNameDecl *New,
// Make the old tag definition visible.
makeMergedDefinitionVisible(Hidden);
- // If this was an unscoped enumeration, yank all of its enumerators
- // out of the scop
https://github.com/cor3ntin edited
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -4034,6 +4034,12 @@ class Sema final : public SemaBase {
void MergeTypedefNameDecl(Scope *S, TypedefNameDecl *New,
LookupResult &OldDecls);
+ /// RetireNodesFromMergedDecl - We have just merged the decl 'New' by making
+ /// another definitio
michael-jabbour-sonarsource wrote:
Gentle ping, and I wish you all a happy new year. :smile:
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c
michael-jabbour-sonarsource wrote:
Thanks for the update @vsapsai. Sounds good. Let me know if there are any
changes that can make reviewing the PR easier!
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists
vsapsai wrote:
Sorry for the delay. Will need to look at the change in a debugger.
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
michael-jabbour-sonarsource wrote:
Gentle ping :smile:
I would love to hear your feedback, and if any changes are needed, I am happy
to address them promptly.
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@l
michael-jabbour-sonarsource wrote:
Gentle ping :smile:
The aim of the PR is to fix an ASTWriter crash that currently happens when
serializing merged enums. Such a case was encountered while parsing Objective-C
code that uses the XCode 16 SDK. If any of the changes look risky, I am happy
to re
jansvoboda11 wrote:
> @jansvoboda11 for OC related things
Thanks! CC @vsapsai and @ian-twilightcoder.
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listi
ChuanqiXu9 wrote:
@jansvoboda11 for OC related things
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
michael-jabbour-sonarsource wrote:
Ping?
Note that this PR fixes a crash in ASTWriter on valid Objective-C modules code;
The initial PR title didn't accurately reflect that.
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing lis
https://github.com/michael-jabbour-sonarsource edited
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/michael-jabbour-sonarsource edited
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/michael-jabbour-sonarsource edited
https://github.com/llvm/llvm-project/pull/114240
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
60 matches
Mail list logo
