-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[libcxx] Add local %T substitution #160026
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
[libcxx] Add local %T substitution #160026
Conversation
Created using spr 1.3.6
@llvm/pr-subscribers-libcxx Author: Aiden Grossman (boomanaiden154) ChangesThis patch adds a %T substitution directly into the libc++ test format. Full diff: https://github.com/llvm/llvm-project/pull/160026.diff 1 Files Affected:
diff --git a/libcxx/utils/libcxx/test/format.py b/libcxx/utils/libcxx/test/format.py
index 5765afec399cf..c9dffd1bb7971 100644
--- a/libcxx/utils/libcxx/test/format.py
+++ b/libcxx/utils/libcxx/test/format.py
@@ -92,6 +92,7 @@ def parseScript(test, preamble):
# errors, which doesn't make sense for clang-verify tests because we may want to check
# for specific warning diagnostics.
_checkBaseSubstitutions(substitutions)
+ substitutions.append(("%T", tmpDir))
substitutions.append(
("%{build}", "%{cxx} %s %{flags} %{compile_flags} %{link_flags} -o %t.exe")
)
|
I'm going to land this given all the CI is passing (minus the flaky arm bots) and that it is pretty trivial. If there is any postcommit followup needed, please let me know and I'll address any comments there. I want to get this in reasonably soon to ensure we don't backslide on having |
This patch adds a %T substitution directly into the libc++ test format. This ensures that the libc++ test format will continue to work when we remove support for %T in llvm lit. Reviewers: #reviewers-libcxx, ldionne, philnik777 Pull Request: llvm/llvm-project#160026
# errors, which doesn't make sense for clang-verify tests because we may want to check | ||
# for specific warning diagnostics. | ||
_checkBaseSubstitutions(substitutions) | ||
substitutions.append(("%T", tmpDir)) |
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 would suggest introducing this as %{temp}
and documenting it officially as a substitution added by the libc++ testing format instead. Once %T
is removed in Lit, this substitution will become seemingly random for anybody who isn't familiar with the current %T
substitution provided by Lit.
This patch adds a %T substitution directly into the libc++ test format. This ensures that the libc++ test format will continue to work when we remove support for %T in llvm lit. Reviewers: #reviewers-libcxx, ldionne, philnik777 Pull Request: llvm#160026
Based on review feedback in llvm#160026. This makes the substitution a lot more clear now that there is no documentation around %T.
Based on review feedback in #160026. This makes the substitution a lot more clear now that there is no documentation around %T. --------- Co-authored-by: Louis Dionne <[email protected]>
Support for the `%T` temp dir substitution is being removed from lit. The libc++ testing format is instead introducing `%{temp}` to replace it, and the downstream test configurations need updating to match. For reference, see: llvm/llvm-project#160026 llvm/llvm-project#162323
Based on review feedback in llvm#160026. This makes the substitution a lot more clear now that there is no documentation around %T. --------- Co-authored-by: Louis Dionne <[email protected]>
Based on review feedback in llvm#160026. This makes the substitution a lot more clear now that there is no documentation around %T. --------- Co-authored-by: Louis Dionne <[email protected]>
This patch adds a %T substitution directly into the libc++ test format.
This ensures that the libc++ test format will continue to work when we
remove support for %T in llvm lit.