[clang] d4af865 - [clangd] Fix type printing in the presence of qualifiers

Adam Czachorowski via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 8 08:06:30 PST 2021


Author: Adam Czachorowski
Date: 2021-01-08T17:00:39+01:00
New Revision: d4af86581e80ef0f7a6f4a4fff1c97260a726e71

URL: https://github.com/llvm/llvm-project/commit/d4af86581e80ef0f7a6f4a4fff1c97260a726e71
DIFF: https://github.com/llvm/llvm-project/commit/d4af86581e80ef0f7a6f4a4fff1c97260a726e71.diff

LOG: [clangd] Fix type printing in the presence of qualifiers

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

Differential Revision: https://reviews.llvm.org/D94259

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp
index 39ab48843b28..16298f3e0326 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -299,19 +299,27 @@ SymbolID getSymbolID(const llvm::StringRef MacroName, const MacroInfo *MI,
   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 isScopeVisible(const DeclContext *DC) const {
+      return DC->Encloses(CurContext);
+    }
+
+  private:
+    const DeclContext *CurContext;
+  };
+  PrintCB PCB(&CurContext);
+  PP.Callbacks = &PCB;
+
   QT.print(OS, PP);
   return OS.str();
 }

diff  --git a/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
index ba783e551e84..6c96ecc2332f 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
@@ -65,6 +65,11 @@ TEST_F(ExpandAutoTypeTest, Test) {
   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_EQ(apply("void ns::Func() { au^to x = new ns::Class::Nested{}; }"),
+            "void ns::Func() { Class::Nested * x = new ns::Class::Nested{}; }");
+
   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};");

diff  --git a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
index 2033b479896b..94cc4b0a0d84 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -97,6 +97,22 @@ void f(const int c) {
 })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"));
 

diff  --git a/clang/include/clang/AST/PrettyPrinter.h b/clang/include/clang/AST/PrettyPrinter.h
index e06b3b8843ce..3baf2b2ba94d 100644
--- a/clang/include/clang/AST/PrettyPrinter.h
+++ b/clang/include/clang/AST/PrettyPrinter.h
@@ -18,6 +18,7 @@
 
 namespace clang {
 
+class DeclContext;
 class LangOptions;
 class SourceManager;
 class Stmt;
@@ -39,6 +40,15 @@ class PrintingCallbacks {
   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, shouldPrintScope should return true on "foo" NamespaceDecl.
+  /// The printing stops at the first isScopeVisible() == true, so there will
+  /// be no calls with outer scopes.
+  virtual bool isScopeVisible(const DeclContext *DC) const { return false; }
 };
 
 /// Describes how types, statements, expressions, and declarations should be

diff  --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index d1882ac1a3b3..25d7874b53fb 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -1225,6 +1225,9 @@ void TypePrinter::AppendScope(DeclContext *DC, raw_ostream &OS,
   if (DC->isFunctionOrMethod())
     return;
 
+  if (Policy.Callbacks && Policy.Callbacks->isScopeVisible(DC))
+    return;
+
   if (const auto *NS = dyn_cast<NamespaceDecl>(DC)) {
     if (Policy.SuppressUnwrittenScope && NS->isAnonymousNamespace())
       return AppendScope(DC->getParent(), OS, NameInScope);


        


More information about the cfe-commits mailing list