sam-mccall wrote:
Thanks for the pointer to 87848 - reverting that one locally doesn't help
though, even in combination with applying #89005 and #89428. So this change
isn't on the critical path to fixing our builds, but still much appreciated and
will take a look now.
---
Unsurprisingly,
jansvoboda11 wrote:
Just so you're aware, it's possible that
https://github.com/llvm/llvm-project/pull/87848 introduced some unexpected
behavior change. It would be good to check if your issues are caused just by
one patch in isolation or by a combination of more patches that landed recently.
sam-mccall wrote:
#89441 fixes our build problems, with or without this patch applied.
It's possible this patch makes things better - I haven't checked for actual
sloc usage yet, just whether the build fails.
So I'll focus on understanding that one and then return here.
(I think I'm close to
jansvoboda11 wrote:
https://github.com/llvm/llvm-project/pull/89441 might be of interest too.
https://github.com/llvm/llvm-project/pull/89005
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
sam-mccall wrote:
Hmm, I locally reverted https://github.com/llvm/llvm-project/pull/87849 and
still see the same issue.
I'll patch #89428 instead, but I don't see how it could do better - it's just
putting the old and new behavior of #87849 behind a flag, right?
In any case I'll try it, and
zygoloid wrote:
> Unfortunately with this patch I'm still seeing the same
> source-location-exhausted error.
Can you try applying #89428 as well? I think we would need both in order to
prune the module map from the serialized SLocEntries.
https://github.com/llvm/llvm-project/pull/89005
ian-twilightcoder wrote:
> Unfortunately with this patch I'm still seeing the same
> source-location-exhausted error. I'm going to try to understand why...
Damn I really thought that was going to be the one. If you have any way I could
reproduce or better write a test that would be great.
sam-mccall wrote:
Unfortunately with this patch I'm still seeing the same
source-location-exhausted error.
I'm going to try to understand why...
https://github.com/llvm/llvm-project/pull/89005
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
sam-mccall wrote:
Ilya is out on vacation, I'm able to test this and will get started on that now
(apologies for delay & thanks for digging into this)
https://github.com/llvm/llvm-project/pull/89005
___
cfe-commits mailing list
ian-twilightcoder wrote:
> Ok _this_ one should fix the logic I think. @ilya-biryukov can you give this
> a try please?
@ilya-biryukov sorry to ping you again, just nobody else knows how to test this.
https://github.com/llvm/llvm-project/pull/89005
https://github.com/ian-twilightcoder edited
https://github.com/llvm/llvm-project/pull/89005
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ian-twilightcoder edited
https://github.com/llvm/llvm-project/pull/89005
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1313,6 +1313,14 @@ OptionalFileEntryRef
HeaderSearch::LookupSubframeworkHeader(
// File Info Management.
//===--===//
+static bool
+headerFileInfoModuleBitsMatchRole(const HeaderFileInfo *HFI,
+
@@ -1313,6 +1313,14 @@ OptionalFileEntryRef
HeaderSearch::LookupSubframeworkHeader(
// File Info Management.
//===--===//
+static bool
+headerFileInfoModuleBitsMatchRole(const HeaderFileInfo *HFI,
+
ian-twilightcoder wrote:
> Also note that `ASTWriter` uses this logic in couple of places to avoid
> serializing unrelated headers:
>
> ```c++
> if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
> continue;
> ```
>
> Since textual headers from other modules have
jansvoboda11 wrote:
Also note that `ASTWriter` uses this logic in couple of places to avoid
serializing unrelated headers:
```c++
if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
continue;
```
Since textual headers from other modules have `isModuleHeader=false` and
ian-twilightcoder wrote:
> As a test, maybe you could probe the resulting PCM with `-module-file-info`.
What would I be looking for? Presumably the problem is we import the same
module twice but fail to find the built pcm in the module cache and so we build
it again? I don't know what else
https://github.com/ian-twilightcoder edited
https://github.com/llvm/llvm-project/pull/89005
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -84,7 +84,9 @@ struct HeaderFileInfo {
LLVM_PREFERRED_TYPE(bool)
unsigned isModuleHeader : 1;
- /// Whether this header is a `textual header` in a module.
+ /// Whether this header is a `textual header` in a module. If a header is
+ /// textual in one module and
ian-twilightcoder wrote:
Ok _this_ one should fix the logic I think. @ilya-biryukov can you give this a
try please?
https://github.com/llvm/llvm-project/pull/89005
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/ian-twilightcoder updated
https://github.com/llvm/llvm-project/pull/89005
>From 0ea2af8066f2fb307f3bd273cf34da82189af0ab Mon Sep 17 00:00:00 2001
From: Ian Anderson
Date: Tue, 16 Apr 2024 17:08:28 -0700
Subject: [PATCH] [clang][modules] HeaderSearch::MarkFileModuleHeader
@@ -1313,6 +1313,14 @@ OptionalFileEntryRef
HeaderSearch::LookupSubframeworkHeader(
// File Info Management.
//===--===//
+static bool
+headerFileInfoModuleBitsMatchRole(const HeaderFileInfo *HFI,
+
@@ -1313,6 +1313,14 @@ OptionalFileEntryRef
HeaderSearch::LookupSubframeworkHeader(
// File Info Management.
//===--===//
+static bool
+headerFileInfoModuleBitsMatchRole(const HeaderFileInfo *HFI,
+
ian-twilightcoder wrote:
> > @ilya-biryukov can you check that this fixes your running out of source
> > location space problem please?
>
> Just tried it. The patch as is did not help. I've also tried changing the
> previous line to `getExistingFileInfo(, /*WantExternal=*/false)` and it
>
jansvoboda11 wrote:
> getExistingFileInfo(, /*WantExternal=*/false)
Until [recently](https://github.com/llvm/llvm-project/pull/87848) that function
still consulted `ExternalSource` for `HeaderFileInfo` that `IsValid &&
!External && !Resolved`. Maybe you want to try the new
ilya-biryukov wrote:
> @ilya-biryukov can you check that this fixes your running out of source
> location space problem please?
Just tried it. The patch as is did not help.
I've also tried changing the previous line to `getExistingFileInfo(,
/*WantExternal=*/false)` and it didn't help either.
jansvoboda11 wrote:
As a test, maybe you could probe the resulting PCM with `-module-file-info`.
https://github.com/llvm/llvm-project/pull/89005
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
ian-twilightcoder wrote:
@ilya-biryukov can you check that this fixes your running out of source
location space problem please?
https://github.com/llvm/llvm-project/pull/89005
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
ian-twilightcoder wrote:
I don't really know a great way to write a test for this. If someone could
point me at a similar existing test or has an idea how to write a new test that
would be much appreciated.
https://github.com/llvm/llvm-project/pull/89005
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Ian Anderson (ian-twilightcoder)
Changes
HeaderSearch::MarkFileModuleHeader is no longer properly checking for
no-changes, and so creates a new HeaderFileInfo for every `textual header`,
causes PCM use to go ballistic.
---
Full diff:
https://github.com/ian-twilightcoder created
https://github.com/llvm/llvm-project/pull/89005
HeaderSearch::MarkFileModuleHeader is no longer properly checking for
no-changes, and so creates a new HeaderFileInfo for every `textual header`,
causes PCM use to go ballistic.
>From
31 matches
Mail list logo
