[PATCH] D148216: Add support for annotations in UpdateTestChecks (NFC)

Mon Jun 19 08:53:29 PDT 2023

nikic added inline comments.

================
Comment at: llvm/utils/UpdateTestChecks/common.py:1286
+      if value == default_value:
+        continue
     if action.dest == 'filters':
----------------
nikic wrote:
> hnrklssn wrote:
> > nikic wrote:
> > > We should also not print the `all` argument for `--check-globals` argument for `version < 3`, otherwise that will introduce a spurious change in all such tests.
> > I don't know how to dynamically change the `--check-globals` option between `store_true` and `choices`, since the behaviour can itself be affected by which argument is passed to the `--version` option. So the way I see it there's no way of knowing how to parse between two different option types ahead of time. For the default when no option is specified the parsing is the same however, so we can simply infer later what option is implied based on the version.
> It's not necessary to change the option, just how it is printed. I.e. add something like this:
> ```
>             if args.version < 3 and value == "all":
>                 # Don't include argument value in older versions.
>                 autogenerated_note_args += "--check-globals "
>                 continue
> ```
Ah, I get what you're saying now -- old UTC_ARGS are not accepted at all anymore, and trying to update existing tests will just fail with an error!

We do need old UTC_ARGS to work. I think you can make the argument value optional using these options:
```
        nargs="?", 
        const="default",
        default="default",
        choices=["none", "smart", "all", "default"],
```

Or possibly omit const/default and handle None instead.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148216/new/

https://reviews.llvm.org/D148216