[PATCH] D69560: [clang-tidy] Add 'bugprone-easily-swappable-parameters' check

Whisperity via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 2 07:13:28 PST 2021


whisperity marked 3 inline comments as done.
whisperity added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:402-403
+
+    // Unfortunately, undersquiggly highlights are for some reason chewed up
+    // and not respected by diagnostics from Clang-Tidy. :(
+    diag(First->getLocation(), "the first parameter in the range is '%0'",
----------------
whisperity wrote:
> aaron.ballman wrote:
> > whisperity wrote:
> > > aaron.ballman wrote:
> > > > Can you post a screenshot of what you mean?
> > > Given
> > > 
> > > ```
> > > Diag << SourceRange{First->getSourceRange().getBegin(), Last->getSourceRange().getEnd()};
> > > ```
> > > 
> > > The diagnostic is still produced with only a `^` at the original `diag(Location)`, ignoring the fed `SourceRange`:
> > > 
> > > ```
> > > /home/whisperity/LLVM/Build/../llvm-project/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp:21:18: warning: 3 adjacent parameters of 'redeclChain' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters]
> > > void redeclChain(int I, int J, int K) {}
> > >                  ^
> > > ```
> > > 
> > > Instead of putting all the squigglies in as the range-location highlight, like how `Sema` can diagnose:
> > > 
> > > ```
> > > /home/whisperity/LLVM/Build/../llvm-project/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp:21:18: warning: 3 adjacent parameters of 'redeclChain' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters]
> > > void redeclChain(int I, int J, int K) {}
> > >                  ^~~~~~~~~~~~~~~~~~~
> > > ```
> > > 
> > > I would not want to put additional note tags in an otherwise already verbose output.
> > > 
> > > Back in 2019 I was investigating this issue specifically for another checker I was working on, but hit the wall... Somewhere deep inside where Tidy diagnostic stuff is translated and consumed to Clang stuff, it goes away. It shouldn't, because there are statements that seemingly copy the `Diag.Ranges` array over, but it still does not come out.
> > > 
> > > ... EDIT: I found [[ http://lists.llvm.org/pipermail/cfe-dev/2019-October/063676.html | the relevant mailing list post ]].
> > > ... EDIT: I found the relevant mailing list post.
> > 
> > Oh wow, I had no idea! That's a rather unfortunate bug. :-(
> Speaking of bugs, shall I put it up on Bugzilla, or see if anyone reported already? Would require me digging into that "where are the data structures copied" kind of thing to figure it out again.
Nevermind, this is worth [[ http://bugzilla.llvm.org/show_bug.cgi?id=49000 | a bug report ]] just so we can track it in the proper location.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69560



More information about the cfe-commits mailing list