hahnjo wrote:
@ilya-biryukov @emaxx-google ping, any updates on this? I would be curious to
learn more details about the "rather special build mode" and, if possible, if
https://github.com/llvm/llvm-project/pull/154158 on its own is good to go
https://github.com/llvm/llvm-project/pull/133057
_
hahnjo wrote:
Hm, then I need more information how to reproduce before I can meaningfully
look into it...
https://github.com/llvm/llvm-project/pull/133057
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/l
@@ -4404,14 +4404,19 @@ CompareImplicitConversionSequences(Sema &S,
SourceLocation Loc,
Result = CompareStandardConversionSequences(S, Loc,
ICS1.Standard, ICS2.Standard);
else if (ICS1.isUserDefined()) {
+const Function
https://github.com/hahnjo updated
https://github.com/llvm/llvm-project/pull/154158
>From 25ca9b45bfad32d0c273950ce5607fc3172d2145 Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld
Date: Tue, 19 Aug 2025 14:21:11 +0200
Subject: [PATCH 1/3] [Sema] Compare canonical conversion function
With lazy temp
hahnjo wrote:
> > Ok, that's progress at least. I will then land the PR with the fix some
> > time soon and we can take that item off the list.
>
> How can we make sure the new reported errors are not triggered by #154158?
https://github.com/llvm/llvm-project/pull/154158 "relaxes" a check so t
hahnjo wrote:
> Yes it seems that the original target in question compiles successfully now.
Ok, that's progress at least. I will then land the PR with the fix some time
soon and we can take that item off the list.
> Also all new errors seem to come from a rather special build mode.
Hm, maybe
hahnjo wrote:
> > Invested some time today and I think the fix is #154158. It works for the
> > reproducer posted in [#133057
> > (comment)](https://github.com/llvm/llvm-project/pull/133057#issuecomment-2886816972),
> > @ilya-biryukov or @emaxx-google if you have some time, would it be
> > po
@@ -0,0 +1,143 @@
+// RUN: rm -rf %t
hahnjo wrote:
Yes, I totally see that this snippet gets us into the code path. However, it's
completely besides the point because the `ConversionFunction`s are already
canonical. In fact, there are only two tests in the repo
https://github.com/hahnjo updated
https://github.com/llvm/llvm-project/pull/154158
>From 25ca9b45bfad32d0c273950ce5607fc3172d2145 Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld
Date: Tue, 19 Aug 2025 14:21:11 +0200
Subject: [PATCH 1/2] [Sema] Compare canonical conversion function
With lazy temp
@@ -0,0 +1,143 @@
+// RUN: rm -rf %t
hahnjo wrote:
If you tell me how? You have modify decls while you are constructing the two
conversion sequences. The only way I think this is possible is with
deserialization.
https://github.com/llvm/llvm-project/pull/15415
hahnjo wrote:
> > With lazy template loading, it is possible to find non-canonical
> > FunctionDecls
>
> But this may be concerning. Do you feel this may cause more problem?
Of course this can result in more problems like the one fixed here. That is the
box we opened with lazy loading of temp
@@ -0,0 +1,143 @@
+// RUN: rm -rf %t
hahnjo wrote:
It is the smallest reproducer we have for the moment. It cannot be integrated
into `overload-decl.cpp` because we need modules to trigger deserialization and
have overload resolution find different `FunctionDec
@@ -4404,14 +4404,19 @@ CompareImplicitConversionSequences(Sema &S,
SourceLocation Loc,
Result = CompareStandardConversionSequences(S, Loc,
ICS1.Standard, ICS2.Standard);
else if (ICS1.isUserDefined()) {
+auto ConvFunc1
https://github.com/hahnjo edited
https://github.com/llvm/llvm-project/pull/154158
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hahnjo wrote:
> Unfortunately, this fix is hard to test in isolation without the changes in
> #133057 that make lazy template loading more likely to complete redecl chains
> at "inconvenient" times. The added reproducer passes before and after this
> commit, but would have failed with the prop
https://github.com/hahnjo ready_for_review
https://github.com/llvm/llvm-project/pull/154158
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/hahnjo updated
https://github.com/llvm/llvm-project/pull/133057
>From e5f64e795f10a81a009b4562db0de692deb4c0f7 Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld
Date: Tue, 25 Mar 2025 11:27:59 +0100
Subject: [PATCH 1/5] [Serialization] Hash inner template arguments
The code is a
https://github.com/hahnjo edited
https://github.com/llvm/llvm-project/pull/154158
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/hahnjo updated
https://github.com/llvm/llvm-project/pull/154158
>From 25ca9b45bfad32d0c273950ce5607fc3172d2145 Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld
Date: Tue, 19 Aug 2025 14:21:11 +0200
Subject: [PATCH] [Sema] Compare canonical conversion function
With lazy template
hahnjo wrote:
Invested some time today and I think the fix is
https://github.com/llvm/llvm-project/pull/154158. It works for the reproducer
posted in
https://github.com/llvm/llvm-project/pull/133057#issuecomment-2886816972,
@ilya-biryukov or @emaxx-google if you have some time, would it be po
https://github.com/hahnjo unassigned
https://github.com/llvm/llvm-project/pull/154158
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/hahnjo created
https://github.com/llvm/llvm-project/pull/154158
This can be triggered with lazy template loading.
Draft because it would be nice to have a standalone test for this...
>From 05dce547dd35e40ddc7ff8f4d788be68d3ddc8a5 Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld
hahnjo wrote:
@michael-jabbour-sonarsource please have a look at
https://llvm.org/docs//DeveloperPolicy.html#github-email-address, there is a
"Keep my email addresses private" setting
https://github.com/llvm/llvm-project/pull/150430
___
cfe-commits m
hahnjo wrote:
> Currently running all regression tests to make sure nothing else breaks.
Yes, all seems to work fine.
> Historically, I believe we may have hit the same problem and our proposal was
> to have additional functions to iterate over already loaded specializations:
> https://review
hahnjo wrote:
> @hahnjo, if we have a working reproducer could we see if @zygoloid's comment
> helps us:
>
> > If this issue is indeed specific to conversion function templates, perhaps
> > a path forward would be to disable some part of the lazy loading for only
> > those templates to unbloc
hahnjo wrote:
I don't think we should merge a partial state of this PR for two reasons: 1.
The three patches alone don't actually bring much benefit. When I measured in
ROOT, we definitely needed the (currently) problematic one for lazy template
loading to become really effective. 2. As commen
https://github.com/hahnjo converted_to_draft
https://github.com/llvm/llvm-project/pull/133057
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hahnjo wrote:
I'm not so sure it's about conversion function templates in particular: if it's
really commit 658d55ba1117a029f37f51a3072585a1fd9fc59f causing the problem
(@emaxx-google if you happen to have some time, it would be great to confirm
reverting that change also avoids the issues in
hahnjo wrote:
Thanks, the reproducer is indeed useful. The first "bad" commit is the second
`Complete only needed partial specializations`.
I was manually able to further reduce the example (throwing out empty files &
modules, inline through typedefs, etc)
Smaller reproducer
```
//--- 2OT.h
@@ -367,12 +367,6 @@ bool RedeclarableTemplateDecl::loadLazySpecializationsImpl(
if (!ExternalSource)
return false;
- // If TPL is not null, it implies that we're loading specializations for
- // partial templates. We need to load all specializations in such cases.
-
https://github.com/hahnjo closed
https://github.com/llvm/llvm-project/pull/137804
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/hahnjo created
https://github.com/llvm/llvm-project/pull/137804
Recently commit dc17429ae6 removed some code related to template arguments in
`NestedNameSpecifier::print` that would not pass on the
`TemplateParameterList`. This would cause `printIntegral` to add type suffixe
hahnjo wrote:
> > As far as I can tell, -fincremental-extensions should set the language
> > option IncrementalExtensions which in turn is the default for
> > Preprocessor::IncrementalProcessing.
>
> It is true that it is the default, but it's possible for clients to have one
> without the ot
@@ -367,12 +367,6 @@ bool RedeclarableTemplateDecl::loadLazySpecializationsImpl(
if (!ExternalSource)
return false;
- // If TPL is not null, it implies that we're loading specializations for
- // partial templates. We need to load all specializations in such cases.
-
@@ -21,17 +21,6 @@ using namespace clang;
namespace {
class TemplateArgumentHasher {
- // If we bail out during the process of calculating hash values for
- // template arguments for any reason. We're allowed to do it since
- // TemplateArgumentHasher are only required to g
@@ -367,12 +367,6 @@ bool RedeclarableTemplateDecl::loadLazySpecializationsImpl(
if (!ExternalSource)
return false;
- // If TPL is not null, it implies that we're loading specializations for
- // partial templates. We need to load all specializations in such cases.
-
hahnjo wrote:
> When I have time, I will need to see that I turn the reduced example into
> actual valid code to understand what is going on...
I spent some time turning it into valid code, but then noticed that the `error:
use of overloaded operator '=' is ambiguous` is gone. Going back, the
hahnjo wrote:
> The simplified script: https://pastebin.com/udVTaPYV
Aaaah, the important line is `EXTRA_CFLAGS='-Xclang
-fallow-pcm-with-compiler-errors -ferror-limit=0'`. With that I indeed get the
following diff between logs from `main` and a rebased version of this branch:
```diff
--- main
hahnjo wrote:
> Here's a new reproducer, this time verifying that a semi-fresh
> tip-of-the-tree doesn't trigger the same error:
> https://pastebin.com/7tYfjazz. Apologies for the delay.
Thanks. I gave it a try, but I don't see any `use of overloaded operator '=' is
ambiguous` error. In fact,
hahnjo wrote:
Hi @emaxx-google, any updates on the second minimization run?
https://github.com/llvm/llvm-project/pull/133057
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hahnjo wrote:
> Maybe you can test it with this and land it with different patches. So that
> we can revert one of them if either of them are problematic but other parts
> are fine.
I'm ok with pushing the commits one-by-one after the PR is reviewed, just let
me know.
> > Complete only neede
hahnjo wrote:
### Performance measurements with LLVM
I tested these patches for building LLVM itself with modules
(`LLVM_ENABLE_MODULES=ON`). To work around
https://github.com/llvm/llvm-project/issues/130795, I apply
https://github.com/llvm/llvm-project/pull/131354 before building Clang. In
hahnjo wrote:
> While I may not able to look into them in detail recently, it may be helpful
> to split this into seperate patches to review and to land.
I initially considered this, but @vgvassilev said in
https://github.com/root-project/root/pull/17722#issuecomment-2706555950 he
prefers a s
https://github.com/hahnjo created
https://github.com/llvm/llvm-project/pull/133057
* Hash inner template arguments: The code is applied from `ODRHash::AddDecl`
with the reasoning given in the comment, to reduce collisions. This was
particularly visible with STL types templated on `std::pair`
hahnjo wrote:
I believe
https://github.com/llvm/llvm-project/commit/30ea0f0ce476bf4c12684a9a514af2ca660bbe44
already addresses the cast. The bots are just slow...
https://github.com/llvm/llvm-project/pull/119333
___
cfe-commits mailing list
cfe-commi
https://github.com/hahnjo closed
https://github.com/llvm/llvm-project/pull/104964
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/hahnjo created
https://github.com/llvm/llvm-project/pull/104964
None
>From dc37b356fb9527c3b4cf6b31f55d2dd5067fc29d Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld
Date: Tue, 20 Aug 2024 16:25:15 +0200
Subject: [PATCH 1/2] [clang-repl] Fix printing preprocessed tokens
---
cl
https://github.com/hahnjo closed
https://github.com/llvm/llvm-project/pull/103028
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hahnjo wrote:
> > BTW, the delayed template parsing is a deprecated technique. Both clang and
> > MSVC won't enable this after C++20 and we think it is the root of many bugs.
>
> I agree. It was needed in the past to parse the MSVC stdlib, let's check if
> we still need it:
> [root-project/ro
https://github.com/hahnjo created
https://github.com/llvm/llvm-project/pull/103028
When instantiating a delayed template, the recorded token stream is passed to
`Parser::ParseLateTemplatedFuncDef` which will append the current token "so it
doesn't get lost". With incremental extensions enabled
hahnjo wrote:
> > Quick question for my understanding: With `spr` we don't get meaningful
> > commit messages anymore? That's quite unfortunate...
>
> Can you please clarify?
The commit messages in this PR are `initial version` and `Update const.cpp`,
which is totally useless if I came across
hahnjo wrote:
Quick question for my understanding: With `spr` we don't get meaningful commit
messages anymore? That's quite unfortunate...
https://github.com/llvm/llvm-project/pull/102859
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https:/
https://github.com/hahnjo closed
https://github.com/llvm/llvm-project/pull/102450
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hahnjo wrote:
> Your reasoning sounds right to me. Can we make sure we are not breaking
> `clang -fincremental-extensions`, too?
As far as I can tell, `-fincremental-extensions` should set the language option
`IncrementalExtensions` which in turn is the default for
`Preprocessor::IncrementalP
https://github.com/hahnjo created
https://github.com/llvm/llvm-project/pull/102450
When incremental processing is enabled, the Parser will never report `tok::eof`
but `tok::annot_repl_input_end`. However, that case is already taken care of in
`IncrementalParser::ParseOrWrapTopLevelDecl()` so t
https://github.com/hahnjo closed https://github.com/llvm/llvm-project/pull/76473
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -364,6 +365,19 @@ IncrementalParser::Parse(llvm::StringRef input) {
std::unique_ptr IncrementalParser::GenModule() {
static unsigned ID = 0;
if (CodeGenerator *CG = getCodeGen()) {
+// Clang's CodeGen is designed to work with a single llvm::Module. In many
+// ca
@@ -224,11 +228,8 @@ IncrementalParser::IncrementalParser(Interpreter &Interp,
return; // PTU.takeError();
}
- if (CodeGenerator *CG = getCodeGen()) {
-std::unique_ptr M(CG->ReleaseModule());
-CG->StartModule("incr_module_" + std::to_string(P
Stefan =?utf-8?q?Gr=C3=A4nitz?=
Message-ID:
In-Reply-To:
@@ -14,7 +14,7 @@ struct A { int a; A(int a) : a(a) {} virtual ~A(); };
// PartialTranslationUnit.
inline A::~A() { printf("~A(%d)\n", a); }
-// Create one instance with new and delete it.
+// Create one instance with
Stefan =?utf-8?q?Gränitz?=
Message-ID:
In-Reply-To:
@@ -14,7 +14,7 @@ struct A { int a; A(int a) : a(a) {} virtual ~A(); };
// PartialTranslationUnit.
inline A::~A() { printf("~A(%d)\n", a); }
-// Create one instance with new and delete it.
+// Create one instance with new
Stefan =?utf-8?q?Gränitz?=
Message-ID:
In-Reply-To:
@@ -14,7 +14,7 @@ struct A { int a; A(int a) : a(a) {} virtual ~A(); };
// PartialTranslationUnit.
inline A::~A() { printf("~A(%d)\n", a); }
-// Create one instance with new and delete it.
+// Create one instance with new
@@ -2,56 +2,56 @@
// REQUIRES: nvptx-registered-target
// REQUIRES: zlib
-// RUN: not %clang -### --target=x86_64-linux-gnu -c %s -g -gz 2>&1 \
+// RUN: %clang -### --target=x86_64-linux-gnu --offload-arch=sm_52 -nogpulib
-nogpuinc -c %s -g -gz 2>&1 \
// RUN: | FileCheck %s
hahnjo wrote:
https://github.com/llvm/llvm-project/pull/84017 changed the test in ways that
this isn't needed anymore.
https://github.com/llvm/llvm-project/pull/84008
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-b
https://github.com/hahnjo closed https://github.com/llvm/llvm-project/pull/84008
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -2,56 +2,56 @@
// REQUIRES: nvptx-registered-target
// REQUIRES: zlib
-// RUN: not %clang -### --target=x86_64-linux-gnu -c %s -g -gz 2>&1 \
+// RUN: %clang -### --target=x86_64-linux-gnu --offload-arch=sm_52 -nogpulib
-nogpuinc -c %s -g -gz 2>&1 \
// RUN: | FileCheck %s
hahnjo wrote:
Ok, but that still doesn't change the fact that the Clang driver will search
for a system-wide CUDA installation unless passed `--cuda-path`...
https://github.com/llvm/llvm-project/pull/84008
___
cfe-commits mailing list
cfe-commits@list
hahnjo wrote:
> > > Might need `-nogpulib -nogpuinc` in those cases, we do that in other
> > > `.cu` files in the test suite.
> >
> >
> > No, I already tried that, it doesn't work for me. All
> > `clang/test/Driver/*.cu` that supply `-nocudainc` also pass `--cuda-path`...
>
> The only reason
hahnjo wrote:
> > It definitely doesn't work for the "pure" CUDA invocations, it still finds
> > my local installation and complains. It might work for the OpenMP
> > invocations, but hard to tell for me on a system with CUDA installed. As
> > it's a `.cu` test after all, I think I would prefe
hahnjo wrote:
> We had a lot that were like this previously. Guessing this one slipped
> through because of the `zlib` requirement.
I actually think there are some more left; for example
`clang/test/Driver/cuda-dwarf-2.cu` has many `not %clang` invocations that
don't specify `--cuda-path`. Th
hahnjo wrote:
> But my point is that we can't land that if we don't understand what's going
> wrong without that patch.
We understand that very well and it's described in
https://reviews.llvm.org/D153003 as well as the surrounding discussions:
because of the way that `ODRHash` works, template
hahnjo wrote:
As far as I can tell from
https://github.com/llvm/llvm-project/pull/76774#issuecomment-1914177330 above,
the last push only changed the default value of
`LoadExternalSpecializationsLazily`. In that case, my test results from
https://github.com/llvm/llvm-project/pull/76774#issuec
hahnjo wrote:
The newest version of this patch still doesn't work correctly. Switching the
new `LoadExternalSpecializationsLazily` to disabled by default (somehow the
argument didn't work on its own) instead crashes, most of the cases involving
`MultiOnDiskHashTable`. I suspect some kind of me
hahnjo wrote:
> Would you like to give an reproducer so that I can debug?
Not easily at the moment, unless you want to go through building the entirety
of ROOT :-/
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commi
hahnjo wrote:
> > > I tried applying this patch to ROOT/Cling and it fails to build because
> > > something is of the opinion that `std::is_integral::value` is not
> > > true. I don't have time right now to investigate further, but since this
> > > is the only change I did it is highly likely
hahnjo wrote:
> It seems after this change we started to fail on `LLVM ::
> ExecutionEngine/JITLink/RISCV/ELF_ehframe.s` when using debug libgcxx.
First, sorry for the slow response and breaking the builds with expensive
checks in the first place. I believe the report is actually fully correct
hahnjo wrote:
I tried applying this patch to ROOT/Cling and it fails to build because
something is of the opinion that `std::is_integral::value` is not true. I
don't have time right now to investigate further, but since this is the only
change I did it is highly likely that it's caused by this
https://github.com/hahnjo approved this pull request.
Looks good (sorry, this fell through)
https://github.com/llvm/llvm-project/pull/76218
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
https://github.com/hahnjo closed https://github.com/llvm/llvm-project/pull/69076
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hahnjo wrote:
> Please add a release note
> This change needs a release note. Please add an entry to
> `clang/docs/ReleaseNotes.rst` in the section the most adapted to the change,
> and referencing any Github issue this change fixes. Thanks!
Done.
https://github.com/llvm/llvm-project/pull/69
@@ -15754,10 +15754,18 @@ bool Expr::EvaluateAsInitializer(APValue &Value,
const ASTContext &Ctx,
LValue LVal;
LVal.set(VD);
-if (!EvaluateInPlace(Value, Info, LVal, this,
- /*AllowNonLiteralTypes=*/true) ||
-EStatus.HasSideEffects)
https://github.com/hahnjo updated
https://github.com/llvm/llvm-project/pull/69076
>From a55ca99a373b17501d56d18af9e8aa2dc2cbcea0 Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld
Date: Sat, 14 Oct 2023 20:10:28 +0200
Subject: [PATCH 1/3] Fix crash with modules and constexpr destructor
With modules
hahnjo wrote:
> address my previous comment: [#69076
> (comment)](https://github.com/llvm/llvm-project/pull/69076#issuecomment-1780327252)
I had already expanded the commit message with the full details, now also
copied to the PR summary. Is that sufficient to address the comment?
https://git
https://github.com/hahnjo edited https://github.com/llvm/llvm-project/pull/69076
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
hahnjo wrote:
That test does not exercise an alias argument t
hahnjo wrote:
Well, this patch is up since almost three months now (!). Sure, we can keep
carrying a similar fix downstream, but ideally I would really like to get rid
of as many local changes as possible. That's not possible without proper
review, but the current situation is quite unsatisfac
hahnjo wrote:
Ping, is this ok to be accepted and landed?
> So personally I am fine with the current workaround with a `FIXME`.
You mean next to the comment I already added referring to the C++ standard? Can
you formulate what I should put there?
https://github.com/llvm/llvm-project/pull/6907
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
hahnjo wrote:
The review related to `ODRHash` is this one: ht
hahnjo wrote:
I finally had time to debug this: The reason for modules being involved here is
because the serialization code calls `ParmVarDecl::getDefaultArg()` which
strips the outermost `FullExpr`, such as `ExprWithCleanups`. A potential fix is
in https://github.com/llvm/llvm-project/pull/7
https://github.com/hahnjo updated
https://github.com/llvm/llvm-project/pull/69076
>From a55ca99a373b17501d56d18af9e8aa2dc2cbcea0 Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld
Date: Sat, 14 Oct 2023 20:10:28 +0200
Subject: [PATCH] Fix crash with modules and constexpr destructor
With modules, se
https://github.com/hahnjo created
https://github.com/llvm/llvm-project/pull/76473
`ParmVarDecl::getDefaultArg()` strips the outermost `FullExpr`, such as
`ExprWithCleanups`. This leads to an `llvm_unreachable` being executed with the
added test `clang/test/Modules/pr68702.cpp`; instead use the
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
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();
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 bec
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`.
>Fro
hahnjo wrote:
ping @shafik @cor3ntin @ChuanqiXu9, how can we make progress here?
https://github.com/llvm/llvm-project/pull/69076
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/hahnjo approved this pull request.
Looks reasonable to me. I know this fixes a test error for MinGW, but if
possible maybe let it sit until early next week in case somebody else has a
different opinion on moving `host=` to `lit`.
https://github.com/llvm/llvm-project/pull/711
https://github.com/hahnjo approved this pull request.
Very interesting... See also https://github.com/llvm/llvm-project/issues/68092,
now I understand even less what the problem is...
https://github.com/llvm/llvm-project/pull/70991
___
cfe-commits mai
hahnjo wrote:
I can add the comment as requested, but for the other questions related to full
expressions and modules I'd really need input from experts...
https://github.com/llvm/llvm-project/pull/69076
___
cfe-commits mailing list
cfe-commits@lists.
https://github.com/hahnjo closed https://github.com/llvm/llvm-project/pull/68253
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hahnjo wrote:
ping @cor3ntin @shafik, could you have a look here?
https://github.com/llvm/llvm-project/pull/69076
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
1 - 100 of 339 matches
Mail list logo
