[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