https://github.com/vsapsai approved this pull request.
Ok to merge after fixing the formatting.
https://github.com/llvm/llvm-project/pull/137675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe
https://github.com/vsapsai created
https://github.com/llvm/llvm-project/pull/138227
According to the documentation
> A header declaration that does not contain `exclude` nor `textual` specifies
> a header that contributes to the enclosing module.
Which means that `exclude` and `textual` header
vsapsai wrote:
The change for `exclude` headers was done in
040e12662a674e2ebfc93f86a70eddb7d6fc76da.
https://github.com/llvm/llvm-project/pull/138227
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listi
https://github.com/vsapsai created
https://github.com/llvm/llvm-project/pull/138358
Restore the behavior existing prior to
fe2eefc4718f57e1753f7bd51c158fc03d70b34f. Make reporting of unevaluated
directive source range more consistent and with fewer assumptions. In case of a
failed evaluation
@@ -3311,12 +3314,11 @@ ASTReader::ReadControlBlock(ModuleFile &F,
SignatureBytes.end());
Blob = Blob.substr(ASTFileSignature::size);
-if (ImportedFile.empty()) {
- // Use BaseDirectoryAsWritten to ens
@@ -0,0 +1,62 @@
+/// This tests the expected error case when there is a mismatch between the
pcm dependencies passed in
+/// the command line with `fmodule-file` and whats encoded in the pcm.
+
+/// The steps are:
+/// 1. Build module (A-1) with no dependencies.
+/// 2. Build
@@ -0,0 +1,62 @@
+/// This tests the expected error case when there is a mismatch between the
pcm dependencies passed in
+/// the command line with `fmodule-file` and whats encoded in the pcm.
+
+/// The steps are:
+/// 1. Build module (A-1) with no dependencies.
+/// 2. Build
https://github.com/vsapsai edited
https://github.com/llvm/llvm-project/pull/136612
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/vsapsai commented:
What would happen if both A-1.pcm and A-2.pcm are built for the same files
(e.g., from Sysroot) but have different file names? Not saying it is a valid
use case to support, just curious what would happen.
https://github.com/llvm/llvm-project/pull/136612
__
@@ -62,6 +63,28 @@ DarwinSDKInfo::RelatedTargetVersionMapping::parseJSON(
Min, Max, MinValue, MaximumDeploymentTarget, std::move(Mapping));
}
+static llvm::Triple::OSType parseOS(const llvm::json::Object &Obj) {
+ // The CanonicalName is the Xcode platform, a version, a
@@ -2990,26 +2996,18 @@ static bool sdkSupportsBuiltinModules(
return false;
VersionTuple SDKVersion = SDKInfo->getVersion();
- switch (TargetPlatform) {
+ switch (SDKInfo->getOS()) {
// Existing SDKs added support for builtin modules in the fall
// 2024 major re
https://github.com/vsapsai closed
https://github.com/llvm/llvm-project/pull/138227
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
vsapsai wrote:
Thanks for the review!
https://github.com/llvm/llvm-project/pull/138227
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
vsapsai wrote:
> I know what we do is cheesy and we might need to reconsider how to approach
> it, but this is a breaking change and it would be great to find a resolution
> for it before committing to the new behavior. Could we revert this while we
> discuss the potential ways out of it?
Wha
vsapsai wrote:
Do you include proto headers directly or through the wrapper headers?
Are there any important macros in proto headers?
Do you need to abuse macros to get something out of proto headers? Kinda like
`#define private public` but more realistic.
My initial reaction is to offer a so
vsapsai wrote:
Interesting, changing `private header "wrap_foo.h"` to `header "wrap_foo.h"` in
`b.wrap_foo` stops reproducing the error. I'm not sure it guarantees
> whenever resolving module for the header, pick one that has the header as
> modular over textual.
But seems we are closer to th
vsapsai wrote:
> @vsapsai I guess, it's a good sign? :) Do you see how our use case can be
> supported by a trivial and low-risk forward fix? If not, I'd insist on a
> revert before we can figure out the way forward. We can run this sort of a
> change through our testing before it relands, and
vsapsai wrote:
> It is concerning to me that we as a company are given **a week** to adjust to
> the change and there is no discussion either about alternative approaches or
> the scale of the changes needed to support this. We do rely on all kinds of
> weird behaviors and side effects in Clan
vsapsai wrote:
> Back to the original issue.
Do you control the module map that specifies a header as private? Does it need
to stay private?
Personally, to me it looks like a bug that a private header is getting picked
up [somewhat accidentally]. I'm not planning to fix this bug, just that re
Author: Volodymyr Sapsai
Date: 2025-05-22T18:34:30-07:00
New Revision: 720014f70841f0284d21ef8100c406d6c864ac9c
URL:
https://github.com/llvm/llvm-project/commit/720014f70841f0284d21ef8100c406d6c864ac9c
DIFF:
https://github.com/llvm/llvm-project/commit/720014f70841f0284d21ef8100c406d6c864ac9c.di
vsapsai wrote:
Ok, I'm going to revert the change to help you out. But I'm going to re-land it
in a week or when you are ready, whichever comes first.
There was no indication there is anything wrong with the change or if the issue
is wide-spread. And if a single company relies on an existing s
vsapsai wrote:
It's weird that clang picked `b.wrap_foo` for "wrap_foo.h" as it is specified
as `private header`. To me it looks like circumventing "use of private header
from outside its module [-Wprivate-header]".
https://github.com/llvm/llvm-project/pull/138227
_
vsapsai wrote:
Right now I don't have a strong opinion about the specific terminology. Though
it might change as I think about it more.
What I do have a strong opinion about is mentioning a specific file. In the
first quick pass it looks like we do that already, e.g., "please rebuild
precompi
vsapsai wrote:
I think in the second case we usually use "AST file" to lump together .pcm and
.pch.
https://github.com/llvm/llvm-project/pull/142161
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinf
https://github.com/vsapsai approved this pull request.
Discussed it with Cyndy more and I think that the trade-off is
* "AST file" is easier to search for as it is more specific
* "AST file" can be more confusing for non-compiler developers as they aren't
required to work with and to know about
vsapsai wrote:
I'll be busy during WWDC week too, so no real changes are expected in the next
2 weeks.
https://github.com/llvm/llvm-project/pull/138227
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/list
vsapsai wrote:
How natural is it to assume that "precompiled file" is the same-ish as
"compiled file" aka "object file"? I'm wary of making users pay attention to
such details. "Precompiled header" is an existing term, so I'm fine using it.
But it is worth considering what are the differences
vsapsai wrote:
> Thanks a lot, I am sorry for misreading or misunderstanding your comments
> too. I hope we can put this behind ourselves.
Appreciate the understanding.
> [skip the technical description]
Thanks for the explanation, it gives a lot of food for thought. Still building
a mental
vsapsai wrote:
I think I understand the logic behind your approach.
* Access between headers is enforced through decl-use, so you cannot access
headers from a target that's not your direct dependency.
* But some headers shouldn't be accessed outside of a target [regardless of the
dependencies],
vsapsai wrote:
What about the approach to provide all_textual.pcm to use.cpp and for clang to
find foo.h in all_textual.pcm? It doesn't work right now but I'm not sure that
it shouldn't (there can be good reasons why it shouldn't work, tbh).
https://github.com/llvm/llvm-project/pull/138227
___
https://github.com/vsapsai created
https://github.com/llvm/llvm-project/pull/146976
Otherwise we are continuing in an invalid state and can easily crash.
It is a follow-up to cde90e68f8123e7abef3f9e18d79980aa19f460a but an important
difference is when a failure happens in a submodule. In this
@@ -4589,6 +4589,9 @@ bool Lexer::LexDependencyDirectiveToken(Token &Result) {
if (Result.is(tok::hash) && Result.isAtStartOfLine()) {
PP->HandleDirective(Result);
+if (PP->hadModuleLoaderFatalFailure())
+ // With a fatal failure in the module loader, we abort p
vsapsai wrote:
> Seems like this should have a release note?
What is the bar for a change to be release noted? This change is pretty much
"make clang-scan-deps less likely to crash" and personally I don't think it
deserves a release note.
https://github.com/llvm/llvm-project/pull/146976
_
301 - 333 of 333 matches
Mail list logo
