adamcz created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
adamcz requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.
When printing QualType with qualifiers like "const", or pointing to an
elaborated type, we would print garbage like:
std::const std::vector<int>&
with the initial std:: being calculated correctly, but inserted in the
wrong place and the second std:: not removed (due to elaborated type).
This affected, among others, ExtractFunction and ExpandAuto tweaks.
This change introduces a new callback to PrintingPolicy, which allows us
to influence the printing of namespace qualifiers. In the future, the
same callback can be used to improve handling of "using namespace"
directives as well.
Fixes:
https://github.com/clangd/clangd/issues/640 (ExtractFunction)
https://github.com/clangd/clangd/issues/264 (ExpandAuto)
First point of https://github.com/clangd/clangd/issues/524
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D94259
Files:
clang-tools-extra/clangd/AST.cpp
clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
clang/include/clang/AST/PrettyPrinter.h
clang/lib/AST/TypePrinter.cpp
Index: clang/lib/AST/TypePrinter.cpp
===================================================================
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1229,6 +1229,9 @@
if (Policy.SuppressUnwrittenScope && NS->isAnonymousNamespace())
return AppendScope(DC->getParent(), OS, NameInScope);
+ if (Policy.Callbacks && Policy.Callbacks->isNamespaceVisible(NS))
+ return;
+
// Only suppress an inline namespace if the name has the same lookup
// results in the enclosing namespace.
if (Policy.SuppressInlineNamespace && NS->isInline() && NameInScope &&
Index: clang/include/clang/AST/PrettyPrinter.h
===================================================================
--- clang/include/clang/AST/PrettyPrinter.h
+++ clang/include/clang/AST/PrettyPrinter.h
@@ -19,6 +19,7 @@
namespace clang {
class LangOptions;
+class NamespaceDecl;
class SourceManager;
class Stmt;
class TagDecl;
@@ -39,6 +40,15 @@
virtual std::string remapPath(StringRef Path) const {
return std::string(Path);
}
+
+ /// When printing type to be inserted into code in specific context, this
+ /// callback can be used to avoid printing the redundant part of the
+ /// qualifier. For example, when inserting code inside namespace foo, we
+ /// should print bar::SomeType instead of foo::bar::SomeType.
+ /// To do this, isNamespaceVisible should return true on "foo" NamespaceDecl.
+ virtual bool isNamespaceVisible(const NamespaceDecl *NS) const {
+ return false;
+ }
};
/// Describes how types, statements, expressions, and declarations should be
Index: clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -97,6 +97,22 @@
})cpp";
EXPECT_EQ(apply(ConstCheckInput), ConstCheckOutput);
+ // Check const qualifier with namespace
+ std::string ConstNamespaceCheckInput = R"cpp(
+namespace X { struct Y { int z; }; }
+int f(const X::Y &y) {
+ [[return y.z + y.z;]]
+})cpp";
+ std::string ConstNamespaceCheckOutput = R"cpp(
+namespace X { struct Y { int z; }; }
+int extracted(const X::Y &y) {
+return y.z + y.z;
+}
+int f(const X::Y &y) {
+ return extracted(y);
+})cpp";
+ EXPECT_EQ(apply(ConstNamespaceCheckInput), ConstNamespaceCheckOutput);
+
// Don't extract when we need to make a function as a parameter.
EXPECT_THAT(apply("void f() { [[int a; f();]] }"), StartsWith("fail"));
Index: clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
@@ -65,6 +65,9 @@
EXPECT_EQ(apply(R"cpp(au^to x = "test";)cpp"),
R"cpp(const char * x = "test";)cpp");
+ EXPECT_EQ(apply("ns::Class * foo() { au^to c = foo(); }"),
+ "ns::Class * foo() { ns::Class * c = foo(); }");
+
EXPECT_UNAVAILABLE("dec^ltype(au^to) x = 10;");
// expanding types in structured bindings is syntactically invalid.
EXPECT_UNAVAILABLE("const ^auto &[x,y] = (int[]){1,2};");
Index: clang-tools-extra/clangd/AST.cpp
===================================================================
--- clang-tools-extra/clangd/AST.cpp
+++ clang-tools-extra/clangd/AST.cpp
@@ -299,19 +299,27 @@
return SymbolID(USR);
}
-// FIXME: This should be handled while printing underlying decls instead.
std::string printType(const QualType QT, const DeclContext &CurContext) {
std::string Result;
llvm::raw_string_ostream OS(Result);
- auto Decls =
- explicitReferenceTargets(DynTypedNode::create(QT), DeclRelation::Alias);
- if (!Decls.empty())
- OS << getQualification(CurContext.getParentASTContext(), &CurContext,
- Decls.front(),
- /*VisibleNamespaces=*/llvm::ArrayRef<std::string>{});
PrintingPolicy PP(CurContext.getParentASTContext().getPrintingPolicy());
- PP.SuppressScope = true;
PP.SuppressTagKeyword = true;
+ PP.SuppressUnwrittenScope = true;
+
+ class PrintCB : public PrintingCallbacks {
+ public:
+ PrintCB(const DeclContext *CurContext) : CurContext(CurContext) {}
+ virtual ~PrintCB() {}
+ virtual bool isNamespaceVisible(const NamespaceDecl *NS) const {
+ return NS->Encloses(CurContext);
+ }
+
+ private:
+ const DeclContext *CurContext;
+ };
+ PrintCB PCB(&CurContext);
+ PP.Callbacks = &PCB;
+
QT.print(OS, PP);
return OS.str();
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
