diff --git a/clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp b/clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp +++ b/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,62 @@ return D; } +using UserDefinedLiteralSet = llvm::SmallPtrSet; + +void handleUserDefinedLiterals( + Decl *D, const UserDefinedLiteralSet &Literals, + std::map &Result, ASTContext &Ctx) { + struct Visitor : RecursiveASTVisitor { + Visitor(const UserDefinedLiteralSet &Literals, + std::map &Result, + ASTContext &Ctx) + : Literals{Literals}, Result{Result}, Ctx{Ctx} {} + + const UserDefinedLiteralSet &Literals; + std::map &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); + return true; + } + } + } + return true; + } + + // Get the closet CompoundStmt parent of DRE + const CompoundStmt *getCompoundParent(DeclRefExpr *DRE) { + DynTypedNodeList Parents = Ctx.getParents(*DRE); + llvm::SmallVector NodesToProcess{Parents.begin(), + Parents.end()}; + while (!NodesToProcess.empty()) { + DynTypedNode Node = NodesToProcess.back(); + NodesToProcess.pop_back(); + if (const auto *CS = Node.get()) + return CS; + Parents = Ctx.getParents(Node); + NodesToProcess.append(Parents.begin(), Parents.end()); + } + return nullptr; + } + }; + + 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 +201,15 @@ // Collect all references to symbols from the namespace for which we're // removing the directive. std::vector IdentsToQualify; + // Collect all user-defined literals from the namespace for which we're + // removing the directive + std::map LiteralsToUsing; + for (auto &D : Inputs.AST->getLocalTopLevelDecls()) { + if (SM.isBeforeInTranslationUnit(D->getBeginLoc(), FirstUsingDirectiveLoc)) + continue; // Directive was not visible before this point. + + UserDefinedLiteralSet Literals; findExplicitReferences( D, [&](ReferenceLoc Ref) { @@ -164,15 +229,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 +261,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 +285,22 @@ // 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->getQualifiedNameAsString()); + UsingDeclarations.pop_back(); + if (auto Err = R.add(tooling::Replacement(SM, Loc.getLocWithOffset(1), 0, + UsingDeclarations))) + return std::move(Err); + } return Effect::mainFileEdit(SM, std::move(R)); } diff --git a/clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp --- a/clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp +++ b/clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp @@ -250,20 +250,33 @@ 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;