ldionne wrote:
> > So the intent of the current ordering was to allow creating a toolchain by
> > placing the libc++ headers alongside Clang, as is typically done (and
> > exceptionally not done on Apple platforms). On Apple platforms, you
> > basically always specify a sysroot, either
dmpolukhin wrote:
> So the intent of the current ordering was to allow creating a toolchain by
> placing the libc++ headers alongside Clang, as is typically done (and
> exceptionally not done on Apple platforms). On Apple platforms, you basically
> always specify a sysroot, either explicitly
ldionne wrote:
> Yes. What I think Dmitry and I are trying to explain is that it is very weird
> that providing an explicit `-isysroot` in the command line is ignored for the
> C++ headers (and only for them). One can start using `-nostdinc` and friends,
> but that mean rebuilding the header
ilg-ul wrote:
Yes, that's why I said that your patch must be elaborated and check for the
presence of `-isysroot`, since the internal variable may come from other
settings.
https://github.com/llvm/llvm-project/pull/80524
___
cfe-commits mailing
drodriguez wrote:
The other place that a default sysroot might come is the CMake option
`DEFAULT_SYSROOT`. In my builds it is empty (the default), but it might be
pointing to something in those xpack builds:
ilg-ul wrote:
If, for any reason, you need the SDK path to be the first in the search list, I
suggest you add an extra condition, the explicit presence of the `-isysroot`,
since it looks like the `Sysroot` variable may come from other configuration
definitions.
ilg-ul wrote:
> I am suspicious of SDKROOT maybe being in the environment. Can you check if
> your environment is providing that value under the hood?
I checked and there is no SDKROOT in the environment.
https://github.com/llvm/llvm-project/pull/80524
drodriguez wrote:
@ilg-ul: It seems that the driver does not receive any `-isysroot`, but it
seems that `cc1` receives `-isysroot
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk`, so I am suspicious of
`SDKROOT` maybe being in the environment. Can you check if your environment is
ilg-ul wrote:
I did a test with this patch, and, as expected, the resulting behaviour is
wrong.
The verbose build of a hello-world test resulted in:
```
[/Users/ilg/Work/xpack-dev-tools-build/clang-18.1.0-1/darwin-x64/application/bin/clang++
simple-hello.cpp -o simple-hello-cpp-one -v -v]
drodriguez wrote:
> With that being said, can you clarify what you mean by "ignoring the
> command-line option", and can you expand on why the current state of upstream
> Clang is broken in your opinion? But for productivity's sake, let's take for
> granted that the canonical state is
drodriguez wrote:
> But after taking a quick look, my impression is that this change reverses the
> bug fixed in #70817.
>
> To recap, that PR preferred the C++ headers available in the toolchain
> distribution over the SDK headers, when the compiler was launched via a
> symbolic link to the
ldionne wrote:
> @ldionne if downstream is catching upstream, I would love to discuss
> rationale behind ignoring command line option. I asked this question several
> times and haven't got answer. Original
> [diff](https://reviews.llvm.org/D89001) that introduced this behavior also
> didn't
ilg-ul wrote:
Sorry, the discussion went too specific for me :-(
But after taking a quick look, my impression is that this change reverses the
bug fixed in https://github.com/llvm/llvm-project/pull/70817.
To recap, that PR preferred the C++ headers available in the toolchain
distribution
dmpolukhin wrote:
@ldionne if downstream is catching upstream, I would love to discuss rationale
behind ignoring command line option. I asked this question several times and
haven't got answer. Original [diff](https://reviews.llvm.org/D89001) that
introduced this behavior also didn't explain
ldionne wrote:
If we do that, we’ll just create churn. It’s a moving target.
You will « fix » upstream Clang to match « the system compiler » temporarily,
but by doing so you’re causing the downstream Clang to ingest that change too
via auto-merging and that means you’ll flip-flop the state
drodriguez wrote:
I agree with Dmitry in that it would be preferred to have the system compiler
behaviour until the system compiler actually changes.
I have tried again with Xcode 14.3.1 (released almost 1 year ago), which
includes `include/c++/v1` side by side with the compiler, and for it,
dmpolukhin wrote:
@ldionne what about fixing upstream behavior to match actual system compiler
behavior now and when/as soon as we have new behavior in Apple compiler fix it
again in newer clang versions? Moreover as I pointed out Apple compiler
behavior is problematic for clang-tools and
ldionne wrote:
Woah, I just checked Xcode 15.2 and you are totally right, it contains
`clang-1500.1.0.2.5` which still has the behavior reversed from upstream. Sorry
for the confusion, I assumed it was out in recent Xcodes cause the change was
made a while back, but it looks like this change
drodriguez wrote:
The `Xcode.app` I was using in the previous comment was 15.1. I tried again the
same steps with the last version I have (Xcode 15.3b1, 15E5178i) and I obtain
the same behaviour.
https://github.com/llvm/llvm-project/pull/80524
___
dmpolukhin wrote:
@ldionne I also haven't see Apple compiler that matches upstream behavior. In
https://reviews.llvm.org/D157283#4648242, you mentioned that "Xcode 15 RC"
should be such compiler and checked it and couple version after that and none
of them matched upstream so I decided to
drodriguez wrote:
> @drodriguez I'm not certain why you think AppleClang prefers `isysroot` over
> toolchain headers -- it might be because we simply don't ship libc++
> toolchain headers anymore and we only ship libc++ SDK headers. But if you try
> installing libc++ headers in the toolchain
ldionne wrote:
@dmpolukhin
Replying to https://reviews.llvm.org/D157283#4648445:
> @ldionne thank you for the reply. Unfortunately current behaviour makes
> problems for clang-tools like clang-tidy and clangd that read CDBs from
> compile_commands.json. They start looking headers relative
ldionne wrote:
In https://reviews.llvm.org/D157283#4648242, I explained that the current
upstream code actually matches exactly what we have downstream. I made sure to
upstream everything to avoid differences after we started shipping libc++ in
the SDK and were done with our transition.
drodriguez wrote:
> I need to take a closer look, since at first reading I can't evaluate the
> consequences, especially if this does change the behaviour when -isysroot is
> **not** provided.
Small detail that I think you are aware: the `-isysroot` can be provided, but
it needs to have the
ilg-ul wrote:
> they change the behaviour you introduced in
> https://github.com/llvm/llvm-project/pull/70817 when -isysroot is provided.
I need to take a closer look, since at first reading I can't evaluate the
consequences, especially if this does change the behaviour when -isysroot is
drodriguez wrote:
@ilg-ul: you might be interested in reviewing this changes, since they change
the behaviour you introduced in #70817 when `-isysroot` is provided.
If some folks at Apple reads this: it would be preferable to have the actual
code instead of trying to guess which is the
llvmbot wrote:
@llvm/pr-subscribers-clang-driver
@llvm/pr-subscribers-clang
Author: Daniel Rodríguez Troitiño (drodriguez)
Changes
The current Apple Clang behaviour is to prefer `-isysroot` vs libc++ headers
side-by-side the compiler. This has been like that for several Xcode versions,
https://github.com/drodriguez created
https://github.com/llvm/llvm-project/pull/80524
The current Apple Clang behaviour is to prefer `-isysroot` vs libc++ headers
side-by-side the compiler. This has been like that for several Xcode versions,
at least since Xcode 14.
The code was originally
28 matches
Mail list logo
