[PATCH] D79380: [clang-tidy] In TransformerClangTidyCheck, support option SourceNamingStyle.

Yitzhak Mandelbaum via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 5 06:25:51 PDT 2020


ymandel added a comment.

In D79380#2020135 <https://reviews.llvm.org/D79380#2020135>, @gribozavr2 wrote:

> > It's only effect is to determine how a given file is related to header files, specifically how to determine that a header "corresponds" to a the file being examined -- that is, it is _this_ file's header rather than some arbitrary header.
>
> ... and the results of that algorithm directly affect how the includes are sorted. We could also conceivably get more styles in IncludeSorter that affect how headers are sorted (for example, related to case sensitivity).


Sure, but the option itself isn't changing the style, even it influences the output.  Agreed, though, that we could concievably leverage this to actually control details of the underlying algorithm, as you suggest. Although, I think the right answer is just to leave it all to clang-format.

> Although I personally don't think this option is misnamed, I can see your point though. But given that the current name is used in many existing checkers, I don't think we should diverge in this patch. I think the right way would be to rename the option consistently everywhere, and to maintain a compatibility alias with a warning for at least one release.

Will do. I'm fine w/ maintaining consistency.

<rant>
I think a lot of my frustration stems from lost time trying to write tests against what I assumed was the behavior and then more time digginging into the code to actually determine the behavior (because it was only vaguely documented).  Worse, IncludeSorter seems to have a bug for a corner case that probably only arisese naturally in tests -- if you add two headers to a file w/o any existing headers, it always adds them in the order recieved and separates them into different blocks. So, this experience left me with likely an overly strong aversion to the name. :)
</rant>


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79380





More information about the cfe-commits mailing list