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

[Markus Böck via Phabricator via cfe-commits]

zero9178 updated this revision to Diff 398962.
zero9178 added a comment.

Addressed reviewer nits:

- Used proper naming conventions
- Adjusted tests


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

https://reviews.llvm.org/D116775

Files:
  clang/lib/Sema/SemaLookup.cpp
  clang/test/SemaCXX/typo-correction.cpp


Index: clang/test/SemaCXX/typo-correction.cpp
===================================================================
--- clang/test/SemaCXX/typo-correction.cpp
+++ clang/test/SemaCXX/typo-correction.cpp
@@ -757,3 +757,34 @@
     b = g_volatile_uchar // expected-error {{did you mean 'g_volatile_char'}}
   };
 }
+
+namespace PR47272
+{
+
+namespace Views {
+int Take(); // expected-note{{'Views::Take' declared here}}
+}
+
+namespace [[deprecated("use Views instead")]] View {
+using Views::Take;
+}
+
+namespace [[deprecated]] A { // expected-note{{'A' has been explicitly marked deprecated here}}
+int pr47272;
+}
+
+namespace B {
+using A::pr47272;  // expected-note{{'B::pr47272' declared here}} expected-warning{{'A' is deprecated}}
+}
+
+namespace [[deprecated]] C {
+using A::pr47272;
+}
+
+void function() {
+  int x = ::Take(); // expected-error{{no member named 'Take' in the global namespace; did you mean 'Views::Take'?}}
+  int y = ::pr47272; // expected-error{{no member named 'pr47272' in the global namespace; did you mean 'B::pr47272'?}}
+}
+}
+
+
Index: clang/lib/Sema/SemaLookup.cpp
===================================================================
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -4307,18 +4307,35 @@
   if (!CList.empty() && !CList.back().isResolved())
     CList.pop_back();
   if (NamedDecl *NewND = Correction.getCorrectionDecl()) {
-    std::string CorrectionStr = Correction.getAsString(SemaRef.getLangOpts());
-    for (TypoResultList::iterator RI = CList.begin(), RIEnd = CList.end();
-         RI != RIEnd; ++RI) {
-      // If the Correction refers to a decl already in the result list,
-      // replace the existing result if the string representation of Correction
-      // comes before the current result alphabetically, then stop as there is
-      // nothing more to be done to add Correction to the candidate set.
-      if (RI->getCorrectionDecl() == NewND) {
-        if (CorrectionStr < RI->getAsString(SemaRef.getLangOpts()))
-          *RI = Correction;
-        return;
-      }
+    auto RI = llvm::find_if(CList, [NewND](const TypoCorrection &TypoCorr) {
+      return TypoCorr.getCorrectionDecl() == NewND;
+    });
+    if (RI != CList.end()) {
+      // The Correction refers to a decl already in the list. No insertion is
+      // necessary and all further cases will return.
+
+      auto IsDeprecated = [](Decl *D) {
+        while (D) {
+          if (D->isDeprecated())
+            return true;
+          D = llvm::dyn_cast_or_null<NamespaceDecl>(D->getDeclContext());
+        }
+        return false;
+      };
+
+      // Prefer non deprecated Corrections over deprecated and only then
+      // sort using an alphabetical order.
+      std::pair<bool, std::string> NewKey = {
+          IsDeprecated(Correction.getFoundDecl()),
+          Correction.getAsString(SemaRef.getLangOpts())};
+
+      std::pair<bool, std::string> PrevKey = {
+          IsDeprecated(RI->getFoundDecl()),
+          RI->getAsString(SemaRef.getLangOpts())};
+
+      if (NewKey < PrevKey)
+        *RI = Correction;
+      return;
     }
   }
   if (CList.empty() || Correction.isResolved())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D116775.398962.patch
Type: text/x-patch
Size: 3167 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220111/84a2d3fd/attachment-0001.bin>