[PATCH] D116775: [clang][#47272] Avoid suggesting deprecated version of a declaration over another in typo correction

Markus Böck via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 7 08:08:28 PST 2022


zero9178 marked 2 inline comments as done.
zero9178 added inline comments.


================
Comment at: clang/lib/Sema/SemaLookup.cpp:4345
+      return;
     }
   }
----------------
Quuxplusone wrote:
> It seems like this code is overdue for a "compare these two corrections" //predicate//. The simple version would be
> ```
> auto IsBetter = [&](const TypoCorrection& TC1, const TypoCorrection& TC2) {
>     std::pair<bool, std::string> Key1 = { IsDeprecated(TC1.getFoundDecl()), TC1.getAsString(SemaRef.getLangOpts()) };
>     std::pair<bool, std::string> Key2 = { IsDeprecated(TC2.getFoundDecl()), TC2.getAsString(SemaRef.getLangOpts()) };
>     return Key1 < Key2;  // prefer non-deprecated, alphabetically earlier declarations
> };
> for (TypoCorrection& TC : CList) {
>     if (IsBetter(Correction, TC))
>         TC = Correction;
> }
> ```
> Actually, you could even replicate the hoisting of the loop-invariant stuff, the way the old code did:
> ```
> std::pair<bool, std::string> CorrectionKey = { IsDeprecated(Correction.getFoundDecl()), Correction.getAsString(SemaRef.getLangOpts()) };
> auto CorrectionIsBetterThan = [&](const TypoCorrection& TC) {
>     std::pair<bool, std::string> Key = { IsDeprecated(TC.getFoundDecl()), TC.getAsString(SemaRef.getLangOpts()) };
>     return CorrectionKey < Key;  // prefer non-deprecated, alphabetically earlier declarations
> };
> for (TypoCorrection& TC : CList) {
>     if (CorrectionIsBetterThan(TC))
>         TC = Correction;
> }
> ```
Thanks I really liked the idea of using std::pairs comparison for that. 
I chose to separate the concerns, by first finding the declaration in the list and only then using the comparison of the found decl and the new correction. 


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

https://reviews.llvm.org/D116775



More information about the cfe-commits mailing list