carlosgalvezp added a comment.
Thanks for the feedback!
> Can you explain the reasoning of why this approach is better than current
> approach where checks can use global
> options(`Options.getLocalOrGlobal("HeaderFileExtensions",
> utils::defaultHeaderFileExtensions())`) to access the same information?
Nice, I wasn't aware of the `getLocalOrGlobal` function. It's unclear to me
why some arguments are available via both command line and config, and others
only via config. If there is no use case for CLI interface I agree the global
option is sufficient, but I would like to properly document it. There's for
example the option `User: user` that is undocumented. Perhaps I can restructure
a bit the section `Configuration files`?
> Some checks also have the global option `ImplementationFileExtensions`,
> what's the proposal with that. Should checks that use this functionality
> carry on like that, use a negative match of this proposed option or should we
> also add that option to the `ClangTidyOptions`.
For consistency I believe it makes sense to add it as well as a global option.
> What is the migration plan for code-bases that currently use the old
> check-based option? We can't break their existing configs, likewise we have
> to be careful of projects with checked-in .clang-tidy files that make use of
> this new option when maintainers may still have an older version of
> clang-tidy.
We would follow a deprecation strategy. For example, we document in every check
that the current options are deprecated, and users should instead use the
global config. The local config can be removed after 2 clang releases (that
would mean clang 18 here). Using `getLocalOrGlobal` should simplify the logic
of supporting the transition I believe.
> Finally, Why would you make the option a semicolon delimited string. The only
> reason that's used for the current `CheckOptions` is there isn't currently a
> better alternative, For `ClangTidyOptions` we could set the type to be an
> `Optional<std::vector<std::string>>`. This would be more natural when writing
> the yaml file.
Absolutely! The current format is very strange to me, but I kept it to keep the
patch as minimal as possible. A vector of strings looks great.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141000/new/
https://reviews.llvm.org/D141000
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
