[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

Whisperity via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 16 11:22:56 PDT 2021


whisperity added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:384
+  // Pad the two strings to the longer length.
+  std::size_t BiggerLength = std::max(Str1.size(), Str2.size());
+  SmallString<32> S1PadE{Str1}, S2PadE{Str2};
----------------
aaron.ballman wrote:
> whisperity wrote:
> > @aaron.ballman I think I'll do something like `if (BiggerLength < Threshold) return false;`, how does that sound? The comparison is moot in that case, imagine having strings of 2 and 4, padded to 4, with a threshold of 6, for example.
> > 
> > That way even if someone accidentally makes `StringRef` overflow the buffer, we'll be safe on the call paths we have here.
> I think that'd make the behavior much more obvious, but should that be `<=`?
No, because being = to the threshold means that the common prefix/suffix is empty string. It'd make variables such as "A" and "X" deemed related. Generally not something we want, because that's too broad of an assumption that they are "meant to be used together". (Generally you shouldn't name variables like so, but still...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97297



More information about the cfe-commits mailing list