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:
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:
> 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
301 - 322 of 322 matches
Mail list logo
