[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions option

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 5 09:25:43 PST 2023


njames93 added a comment.

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?

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`.

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.

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.

  HeaderFileExtensions: [ h, hh, hxx, hpp, "" ]
  # Or
  HeaderFileExtensions:
    - h
    - hh
    - hxx
    - hpp
    - ""




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141000



More information about the cfe-commits mailing list