aaron.ballman 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};
----------------
whisperity wrote:
> 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...)
Ah, that's a good point, thank you!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97297/new/
https://reviews.llvm.org/D97297
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
