v1nh1shungry updated this revision to Diff 483582.
v1nh1shungry added a comment.
Reimplemented.
Note:
Failed to find a way to use `ReferenceLoc` to insert the using-declarations.
Currently I have to visit the `Decl` again and look for `CompoundStmt`, if there
is any target user-defined literal in the `CompoundStmt` I mark it so that
I can add the using-declaration in it later.
In this way this implementation covers many cases, and does nothing for
user-defined
literals used in the top-level. But it can cause redundancy and is too complex.
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,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;
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
@@ -101,6 +101,60 @@
return D;
}
+using UserDefinedLiteralSet = llvm::SmallPtrSet<const NamedDecl *, 4>;
+
+UserDefinedLiteralSet findLiterals(Stmt *S,
+ const UserDefinedLiteralSet &Targets) {
+ struct Visitor : RecursiveASTVisitor<Visitor> {
+ Visitor(const UserDefinedLiteralSet &Targets) : Targets{Targets} {}
+
+ const UserDefinedLiteralSet &Targets;
+ UserDefinedLiteralSet Result;
+
+ bool VisitDeclRefExpr(const DeclRefExpr *E) {
+ const auto *D = E->getFoundDecl();
+ if (D->getDeclName().getNameKind() ==
+ DeclarationName::CXXLiteralOperatorName) {
+ for (const auto *Target : Targets) {
+ if (D == Target) {
+ Result.insert(D);
+ return true;
+ }
+ }
+ }
+ return true;
+ }
+ };
+
+ Visitor V{Targets};
+ V.TraverseStmt(S);
+ return V.Result;
+}
+
+void handleUserDefinedLiterals(
+ Decl *D, const UserDefinedLiteralSet &Literals,
+ std::map<SourceLocation, UserDefinedLiteralSet> &Result) {
+ struct Visitor : RecursiveASTVisitor<Visitor> {
+ Visitor(const UserDefinedLiteralSet &Literals,
+ std::map<SourceLocation, UserDefinedLiteralSet> &Result)
+ : Literals{Literals}, Result{Result} {}
+
+ const UserDefinedLiteralSet &Literals;
+ std::map<SourceLocation, UserDefinedLiteralSet> &Result;
+
+ bool VisitCompoundStmt(CompoundStmt *CS) {
+ auto LiteralsFound = findLiterals(CS, Literals);
+ if (!LiteralsFound.empty())
+ Result[CS->getBeginLoc()].insert(LiteralsFound.begin(),
+ LiteralsFound.end());
+ return true;
+ }
+ };
+
+ Visitor V{Literals, Result};
+ V.TraverseDecl(D);
+}
+
bool RemoveUsingNamespace::prepare(const Selection &Inputs) {
// Find the 'using namespace' directive under the cursor.
auto *CA = Inputs.ASTSelection.commonAncestor();
@@ -144,7 +198,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, UserDefinedLiteralSet> 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 +226,20 @@
// 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) {
+ Literals.insert(T);
return;
+ }
}
SourceLocation Loc = Ref.NameLoc;
if (Loc.isMacroID()) {
@@ -189,11 +256,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);
}
// Remove duplicates.
llvm::sort(IdentsToQualify);
@@ -213,10 +280,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));
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
