nikic added inline comments.
================
Comment at: llvm/utils/UpdateTestChecks/common.py:1286
+ if value == default_value:
+ continue
if action.dest == 'filters':
----------------
nikic wrote:
> 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.
Correction, I think the right options are:
```
nargs="?",
const="all",
default="default",
choices=["none", "smart", "all"],
```
That is, use `"default"` is it's not specified at all, use `"all"` if only
`--check-globals` is passed and also accept `--check-globals none|smart|all`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148216/new/
https://reviews.llvm.org/D148216
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
