[PATCH] D91966: [clangd] AddUsing: Used spelled text instead of type name.

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 23 09:33:43 PST 2020


kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp:268
+                                               SpelledTokens->back());
+      llvm::StringRef NameRef = SpelledRange.text(SM);
+
----------------
what about saving the full range here, and using it's `start` and `length - name.size()` as removal range in `apply`?

similarly we can also use the `range.text` as text to insert. that way we can inline `getNNSLAsString` as it won't be needed elsewhere anymore.


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp:274
+        return false; // What's spelled doesn't match the qualifier.
+      Name = NameRef.str();
     }
----------------
it is not that important, but I suppose `NameRef` is going to be alive during apply, so I suppose we can keep `Name` as a `llvm::StringRef`.


================
Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:2814
+  uu u;
 })cpp"}};
   llvm::StringMap<std::string> EditedFiles;
----------------
Tests don't seem to be covering something like:
```
namespace foo { namespace bar { struct X {}; } }
using namespace foo;
bar::X^ x;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91966



More information about the cfe-commits mailing list