v1nh1shungry updated this revision to Diff 484170.
v1nh1shungry added a comment.
Insert the using-declarations in the outermost `CompoundStmt` instead of
the innermost one. Can reduce some potential rebundancy.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137817/new/
https://reviews.llvm.org/D137817
Files:
clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -250,20 +250,34 @@
foo + 10;
}
)cpp"},
- {// Does not qualify user-defined literals
- R"cpp(
- namespace ns {
- long double operator "" _w(long double);
+ {
+ R"cpp(
+ namespace a {
+ long double operator""_a(long double);
+ inline namespace b {
+ long double operator""_b(long double);
+ } // namespace b
+ } // namespace a
+ using namespace ^a;
+ int main() {
+ 1.0_a;
+ 1.0_b;
}
- using namespace n^s;
- int main() { 1.5_w; }
)cpp",
- R"cpp(
- namespace ns {
- long double operator "" _w(long double);
- }
+ R"cpp(
+ namespace a {
+ long double operator""_a(long double);
+ inline namespace b {
+ long double operator""_b(long double);
+ } // namespace b
+ } // namespace a
- int main() { 1.5_w; }
+ int main() {using a::operator""_a;
+using a::operator""_b;
+
+ 1.0_a;
+ 1.0_b;
+ }
)cpp"}};
for (auto C : Cases)
EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -13,6 +13,7 @@
#include "clang/AST/Decl.h"
#include "clang/AST/DeclBase.h"
#include "clang/AST/DeclCXX.h"
+#include "clang/AST/ParentMapContext.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Tooling/Core/Replacement.h"
@@ -101,6 +102,66 @@
return D;
}
+void handleUserDefinedLiterals(
+ Decl *D, const llvm::SmallPtrSet<const NamedDecl *, 1> &Literals,
+ std::map<SourceLocation, llvm::StringSet<>> &Result, ASTContext &Ctx) {
+ struct Visitor : RecursiveASTVisitor<Visitor> {
+ Visitor(const llvm::SmallPtrSet<const NamedDecl *, 1> &Literals,
+ std::map<SourceLocation, llvm::StringSet<>> &Result,
+ ASTContext &Ctx)
+ : Literals{Literals}, Result{Result}, Ctx{Ctx} {}
+
+ const llvm::SmallPtrSet<const NamedDecl *, 1> &Literals;
+ std::map<SourceLocation, llvm::StringSet<>> &Result;
+ ASTContext &Ctx;
+
+ bool VisitDeclRefExpr(DeclRefExpr *DRE) {
+ const NamedDecl *ND = DRE->getFoundDecl();
+ // If DRE references to a user-defined literal
+ if (ND->getDeclName().getNameKind() ==
+ DeclarationName::CXXLiteralOperatorName) {
+ for (const NamedDecl *Literal : Literals) {
+ if (ND == Literal) {
+ // If the user-defined literal locates in a CompoundStmt,
+ // we mark the CompoundStmt so that we can add the using-declaration
+ // in it later. If not, the user-defined literal is used in the
+ // top-level, it is reasonable to do nothing for it.
+ if (const CompoundStmt *CS = getCompoundParent(DRE))
+ Result[CS->getLBracLoc()].insert(ND->getQualifiedNameAsString());
+ return true;
+ }
+ }
+ }
+ return true;
+ }
+
+ // Get the outermost CompoundStmt parent of DRE
+ const CompoundStmt *getCompoundParent(DeclRefExpr *DRE) {
+ const CompoundStmt *R = nullptr;
+
+ DynTypedNodeList Parents = Ctx.getParents(*DRE);
+ llvm::SmallVector<DynTypedNode> NodesToProcess{Parents.begin(),
+ Parents.end()};
+
+ while (!NodesToProcess.empty()) {
+ DynTypedNode Node = NodesToProcess.back();
+ NodesToProcess.pop_back();
+
+ if (const auto *CS = Node.get<CompoundStmt>())
+ R = CS;
+ if (Node.get<TranslationUnitDecl>())
+ break;
+ Parents = Ctx.getParents(Node);
+ NodesToProcess.append(Parents.begin(), Parents.end());
+ }
+ return R;
+ }
+ };
+
+ Visitor V{Literals, Result, Ctx};
+ V.TraverseDecl(D);
+}
+
bool RemoveUsingNamespace::prepare(const Selection &Inputs) {
// Find the 'using namespace' directive under the cursor.
auto *CA = Inputs.ASTSelection.commonAncestor();
@@ -144,7 +205,15 @@
// Collect all references to symbols from the namespace for which we're
// removing the directive.
std::vector<SourceLocation> IdentsToQualify;
+ // Collect all user-defined literals from the namespace for which we're
+ // removing the directive
+ std::map<SourceLocation, llvm::StringSet<>> LiteralsToUsing;
+
for (auto &D : Inputs.AST->getLocalTopLevelDecls()) {
+ if (SM.isBeforeInTranslationUnit(D->getBeginLoc(), FirstUsingDirectiveLoc))
+ continue; // Directive was not visible before this point.
+
+ llvm::SmallPtrSet<const NamedDecl *, 1> Literals;
findExplicitReferences(
D,
[&](ReferenceLoc Ref) {
@@ -164,15 +233,22 @@
// Avoid adding qualifiers before user-defined literals, e.g.
// using namespace std;
// auto s = "foo"s; // Must not changed to auto s = "foo" std::s;
- // FIXME: Add a using-directive for user-defined literals
- // declared in an inline namespace, e.g.
- // using namespace s^td;
- // int main() { cout << "foo"s; }
- // change to
- // using namespace std::literals;
- // int main() { std::cout << "foo"s; }
- if (Kind == DeclarationName::NameKind::CXXLiteralOperatorName)
+ // Also add a using-declaration to keep codes well-formed, e.g.
+ // using namespace std;
+ // int main() {
+ // auto s = "foo"s;
+ // }
+ // will change to
+ // int main() {
+ // using std::operator""s;
+ // auto s = "foo"s;
+ // }
+ if (Kind == DeclarationName::NameKind::CXXLiteralOperatorName) {
+ // Can't get the AST node from the ReferenceLoc, we have to
+ // traverse D later to handle these user-defined literals
+ Literals.insert(T);
return;
+ }
}
SourceLocation Loc = Ref.NameLoc;
if (Loc.isMacroID()) {
@@ -189,11 +265,11 @@
assert(Loc.isFileID());
if (SM.getFileID(Loc) != SM.getMainFileID())
return; // FIXME: report these to the user as warnings?
- if (SM.isBeforeInTranslationUnit(Loc, FirstUsingDirectiveLoc))
- return; // Directive was not visible before this point.
IdentsToQualify.push_back(Loc);
},
Inputs.AST->getHeuristicResolver());
+ if (!Literals.empty())
+ handleUserDefinedLiterals(D, Literals, LiteralsToUsing, Ctx);
}
// Remove duplicates.
llvm::sort(IdentsToQualify);
@@ -213,10 +289,20 @@
// Produce replacements to add the qualifiers.
std::string Qualifier = printUsingNamespaceName(Ctx, *TargetDirective) + "::";
for (auto Loc : IdentsToQualify) {
- if (auto Err = R.add(tooling::Replacement(Ctx.getSourceManager(), Loc,
+ if (auto Err = R.add(tooling::Replacement(SM, Loc,
/*Length=*/0, Qualifier)))
return std::move(Err);
}
+ // Produce replacements to add the using-declarations for user-defined
+ // literals
+ for (auto &[Loc, Literals] : LiteralsToUsing) {
+ std::string UsingDeclarations;
+ for (const auto &L : Literals)
+ UsingDeclarations += llvm::formatv("using {0};\n", L.getKey());
+ if (auto Err = R.add(tooling::Replacement(SM, Loc.getLocWithOffset(1), 0,
+ UsingDeclarations)))
+ return std::move(Err);
+ }
return Effect::mainFileEdit(SM, std::move(R));
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
