[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