[PATCH] D80584: [utils] change update_test_checks.py use of 'TMP' value names
Sanjay Patel via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 1 03:09:49 PDT 2020
spatel added a comment.
In D80584#2065560 <https://reviews.llvm.org/D80584#2065560>, @jdoerfert wrote:
> This needs a command line flag so people can change the prefix. The default for that flag should be "TMP" to minimize the noise in updates. We also should have a test.
> FWIW, the UTC_FLAGS logic will "remember" the flag chosen by the user so if you need to change from "TMP" to something else it will be recorded and no further interaction will be necessary.
I disagree with adding an option for this. We don't want to turn a bug into a feature and carry that complexity cost for future users forever. The anonymous variable name is an implementation detail of the script that newcomers should not have to think about.
But I understand that test churn causes grief for current users (I'm a big user of the script and take the blame for the bug).
The problem is that this script and -instnamer conflict perfectly on the name "tmp". One option that we didn't consider in the discussion in PR45951: we could leave this script using "TMP" if we change -instnamer to use something else. Does anyone see any legacy constraint on the "tmp" naming in -instnamer?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80584/new/
https://reviews.llvm.org/D80584
More information about the llvm-commits
mailing list