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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to