-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[clang] Ensure --print-runtime-dir path exists
#102834
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Before this PR, `clang --print-runtime-dir` used to report a non-existant directory if LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF. We now check if any of the known runtime directories exist before printing on stdout.
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Alexandre Ganea (aganea) ChangesBefore this PR, We now check if any of the known runtime directories exist before printing on stdout. If it doesn't, we print Full diff: https://github.com/llvm/llvm-project/pull/102834.diff 1 Files Affected:
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index f4e909b79389bc..4c8cd36dd118ee 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -2230,10 +2230,14 @@ bool Driver::HandleImmediateArgs(Compilation &C) {
}
if (C.getArgs().hasArg(options::OPT_print_runtime_dir)) {
- if (std::optional<std::string> RuntimePath = TC.getRuntimePath())
- llvm::outs() << *RuntimePath << '\n';
- else
- llvm::outs() << TC.getCompilerRTPath() << '\n';
+ for (auto RuntimePath :
+ {TC.getRuntimePath(), std::make_optional(TC.getCompilerRTPath())}) {
+ if (RuntimePath && getVFS().exists(*RuntimePath)) {
+ llvm::outs() << *RuntimePath << '\n';
+ return false;
+ }
+ }
+ llvm::outs() << "(runtime dir is not present)" << '\n';
return false;
}
|
|
With this interpretation, Perhaps the |
|
To give the whole picture, the story is that while building the LLVM package on Windows, I was seeing this: This is, we don't set I think we want to keep the dynamic aspect on supporting both |
`clang -print-runtime-dir` reports a non-existent directory as we build with `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF`. See llvm/llvm-project#102834. I suspect llvm/llvm-project@b6a1473 caused the change by chaning the code in Driver.cpp to not check whether the printed directory existed.
`clang -print-runtime-dir` reports a non-existent directory as we build with `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF`. See llvm/llvm-project#102834. I suspect llvm/llvm-project@b6a1473 caused the change by chaning the code in Driver.cpp to not check whether the printed directory existed.
`clang -print-runtime-dir` reports a non-existent directory as we build with `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF`. See llvm/llvm-project#102834. I suspect llvm/llvm-project@b6a1473 caused the change by chaning the code in Driver.cpp to not check whether the printed directory existed.
`clang -print-runtime-dir` reports a non-existent directory as we build with `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF`. See llvm/llvm-project#102834. I suspect llvm/llvm-project@b6a1473 caused the change by chaning the code in Driver.cpp to not check whether the printed directory existed.
`clang -print-runtime-dir` reports a non-existent directory as we build with `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF`. See llvm/llvm-project#102834. I suspect llvm/llvm-project@b6a1473 caused the change by chaning the code in Driver.cpp to not check whether the printed directory existed.
`clang -print-runtime-dir` reports a non-existent directory as we build with `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF`. See llvm/llvm-project#102834. I suspect llvm/llvm-project@b6a1473 caused the change by chaning the code in Driver.cpp to not check whether the printed directory existed.
`clang -print-runtime-dir` reports a non-existent directory as we build with `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF`. See llvm/llvm-project#102834. I suspect llvm/llvm-project@b6a1473 caused the change by chaning the code in Driver.cpp to not check whether the printed directory existed.
`clang -print-runtime-dir` reports a non-existent directory as we build with `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF`. See llvm/llvm-project#102834. I suspect llvm/llvm-project@b6a1473 caused the change by chaning the code in Driver.cpp to not check whether the printed directory existed.
`clang -print-runtime-dir` reports a non-existent directory as we build with `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF`. See llvm/llvm-project#102834. I suspect llvm/llvm-project@b6a1473 caused the change by chaning the code in Driver.cpp to not check whether the printed directory existed.
`clang -print-runtime-dir` reports a non-existent directory as we build with `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF`. See llvm/llvm-project#102834. I suspect llvm/llvm-project@b6a1473 caused the change by chaning the code in Driver.cpp to not check whether the printed directory existed.
`clang -print-runtime-dir` reports a non-existent directory as we build with `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF`. See llvm/llvm-project#102834. I suspect llvm/llvm-project@b6a1473 caused the change by chaning the code in Driver.cpp to not check whether the printed directory existed.
`clang -print-runtime-dir` reports a non-existent directory as we build with `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF`. See llvm/llvm-project#102834. I suspect llvm/llvm-project@b6a1473 caused the change by chaning the code in Driver.cpp to not check whether the printed directory existed.
`clang -print-runtime-dir` reports a non-existent directory as we build with `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF`. See llvm/llvm-project#102834. I suspect llvm/llvm-project@b6a1473 caused the change by chaning the code in Driver.cpp to not check whether the printed directory existed.
`clang -print-runtime-dir` reports a non-existent directory as we build with `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF`. See llvm/llvm-project#102834. I suspect llvm/llvm-project@b6a1473 caused the change by chaning the code in Driver.cpp to not check whether the printed directory existed.
|
Ping. This still occurs as of today on main, What is it that we want to do here? Add a dynamic behavior as in this PR, that checks if the path is there, and adjust accordingly? Rename the "windows" component to instead use the target triple, as the other plateforms (this seems to be some work). Or conversly, accept that on Windows the path is "windows" and not the target triple? Anything else? |
rnk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found this at the bottom of my inbox after a long cleanout.
I'm struggling to remember it at this point in the evening, but we've done things like this before as part of the runtime path changes, and I don't see any reason to object to this one.
Part of me wants to test this behavior since we have the indirection layer, we don't have to copy the clang binary and create an entire runtime tree in the test directory, but I wouldn't block this over that concern.
In conclusion, I think we should merge the patch.
Before this PR, `clang --print-runtime-dir` on Windows used to report a non-existent directory if `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF`. We now check if any of the known runtime directories exist before printing any of them on stdout. If none exists, we print `(runtime dir is not present)`.
Before this PR, `clang --print-runtime-dir` on Windows used to report a non-existent directory if `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF`. We now check if any of the known runtime directories exist before printing any of them on stdout. If none exists, we print `(runtime dir is not present)`.
Before this PR,
clang --print-runtime-diron Windows used to report a non-existent directory ifLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF.We now check if any of the known runtime directories exist before printing any of them on stdout. If none exists, we print
(runtime dir is not present).