[PATCH] D151785: [clangd] Desugar template parameter aliases in type hints

[Kirill Stoimenov via Phabricator via cfe-commits]

kstoimenov added a comment.

In D151785#4418037 , @zyounan wrote:

> In D151785#4417973 , @kstoimenov 
> wrote:
>
>> Looks like this might have broken a couple of sanitizer builds. Here is one 
>> of them: https://lab.llvm.org/buildbot/#/builders/5/builds/34387. Could you 
>> please revert or fix? 
>> Thanks!
>
> Sorry for the inconvenience. I think this issue had been fixed by 
> https://github.com/llvm/llvm-project/commit/0e8384a0fe4f03d60cd92aba1cae074512481ca2.
>  Does it work now? Please let me know if we still have problems.

The bot went green after the fix. Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151785/new/

https://reviews.llvm.org/D151785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151785: [clangd] Desugar template parameter aliases in type hints

[Younan Zhang via Phabricator via cfe-commits]

zyounan added a comment.

In D151785#4417973 , @kstoimenov 
wrote:

> Looks like this might have broken a couple of sanitizer builds. Here is one 
> of them: https://lab.llvm.org/buildbot/#/builders/5/builds/34387. Could you 
> please revert or fix? 
> Thanks!

Sorry for the inconvenience. I think this issue had been fixed by 
https://github.com/llvm/llvm-project/commit/0e8384a0fe4f03d60cd92aba1cae074512481ca2.
 Does it work now? Please let me know if we still have problems.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151785/new/

https://reviews.llvm.org/D151785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151785: [clangd] Desugar template parameter aliases in type hints

[Kirill Stoimenov via Phabricator via cfe-commits]

kstoimenov added a comment.

Looks like this might have broken a couple of sanitizer builds. Here is one of 
them: https://lab.llvm.org/buildbot/#/builders/5/builds/34387. Could you please 
revert or fix? 
Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151785/new/

https://reviews.llvm.org/D151785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151785: [clangd] Desugar template parameter aliases in type hints

[Younan Zhang via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7d68f2ef411e: [clangd] Desugar template parameter aliases in 
type hints (authored by zyounan).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151785/new/

https://reviews.llvm.org/D151785

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1417,6 +1417,82 @@
   ExpectedHint{": int", "h"}, ExpectedHint{": int", "i"});
 }
 
+TEST(TypeHints, SubstTemplateParameterAliases) {
+  assertTypeHints(
+  R"cpp(
+  template  struct allocator {};
+
+  template 
+  struct vector_base {
+using pointer = T*;
+  };
+
+  template 
+  struct internal_iterator_type_template_we_dont_expect {};
+
+  struct my_iterator {};
+
+  template >
+  struct vector : vector_base {
+using base = vector_base;
+typedef T value_type;
+typedef base::pointer pointer;
+using allocator_type = A;
+using size_type = int;
+using iterator = internal_iterator_type_template_we_dont_expect;
+using non_template_iterator = my_iterator;
+
+value_type& operator[](int index) { return elements[index]; }
+const value_type& at(int index) const { return elements[index]; }
+pointer data() { return &elements[0]; }
+allocator_type get_allocator() { return A(); }
+size_type size() const { return 10; }
+iterator begin() { return iterator(); }
+non_template_iterator end() { return non_template_iterator(); }
+
+T elements[10];
+  };
+
+  vector array;
+
+  auto $no_modifier[[by_value]] = array[3];
+  auto* $ptr_modifier[[ptr]] = &array[3];
+  auto& $ref_modifier[[ref]] = array[3];
+  auto& $at[[immutable]] = array.at(3);
+
+  auto $data[[data]] = array.data();
+  auto $allocator[[alloc]] = array.get_allocator();
+  auto $size[[size]] = array.size();
+  auto $begin[[begin]] = array.begin();
+  auto $end[[end]] = array.end();
+
+
+  // If the type alias is not of substituted template parameter type,
+  // do not show desugared type.
+  using VeryLongLongTypeName = my_iterator;
+  using Short = VeryLongLongTypeName;
+
+  auto $short_name[[my_value]] = Short();
+
+  // Same applies with templates.
+  template 
+  using basic_static_vector = vector;
+  template 
+  using static_vector = basic_static_vector>;
+
+  auto $vector_name[[vec]] = static_vector();
+  )cpp",
+  ExpectedHint{": int", "no_modifier"},
+  ExpectedHint{": int *", "ptr_modifier"},
+  ExpectedHint{": int &", "ref_modifier"},
+  ExpectedHint{": const int &", "at"}, ExpectedHint{": int *", "data"},
+  ExpectedHint{": allocator", "allocator"},
+  ExpectedHint{": size_type", "size"}, ExpectedHint{": iterator", "begin"},
+  ExpectedHint{": non_template_iterator", "end"},
+  ExpectedHint{": Short", "short_name"},
+  ExpectedHint{": static_vector", "vector_name"});
+}
+
 TEST(DesignatorHints, Basic) {
   assertDesignatorHints(R"cpp(
 struct S { int x, y, z; };
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -11,10 +11,12 @@
 #include "HeuristicResolver.h"
 #include "ParsedAST.h"
 #include "SourceCode.h"
+#include "clang/AST/ASTDiagnostic.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Type.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/ScopeExit.h"
@@ -190,6 +192,64 @@
   return Designators;
 }
 
+// Determines if any intermediate type in desugaring QualType QT is of
+// substituted template parameter type. Ignore pointer or reference wrappers.
+bool isSugaredTemplateParameter(QualType QT) {
+  static auto PeelWrappers = [](QualType QT) {
+// Neither `PointerType` nor `ReferenceType` is considered as sugared
+// type. Peel it.
+QualType Next;
+while (!(Next = QT->getPointeeType()).isNull())
+  QT = Next;
+return QT;
+  };
+  while (true) {
+QualType Desugared =
+PeelWrappers(QT->getLocallyUnqualifiedSingleStepDesugaredType());
+if (Desugared == QT)
+  break;
+if (Desugared->getAs())
+  return true;
+QT = Desugared;
+  }
+  return false;
+}
+
+// A simple wrapper for `clang::desugarForDiagnostic` that provides optional
+// semantic.
+std::optional desugar(ASTContext &AST, QualType QT) {
+  bool ShouldAKA;
+  auto Desugared = clang::desugarForDiagnostic(AST, QT, ShouldAKA);
+  if (!ShouldAKA)
+return std::nullopt;
+  re

[PATCH] D151785: [clangd] Desugar template parameter aliases in type hints

[Younan Zhang via Phabricator via cfe-commits]

zyounan updated this revision to Diff 530851.
zyounan added a comment.

Oops. Remove extra debugging statement


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151785/new/

https://reviews.llvm.org/D151785

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1417,6 +1417,82 @@
   ExpectedHint{": int", "h"}, ExpectedHint{": int", "i"});
 }
 
+TEST(TypeHints, SubstTemplateParameterAliases) {
+  assertTypeHints(
+  R"cpp(
+  template  struct allocator {};
+
+  template 
+  struct vector_base {
+using pointer = T*;
+  };
+
+  template 
+  struct internal_iterator_type_template_we_dont_expect {};
+
+  struct my_iterator {};
+
+  template >
+  struct vector : vector_base {
+using base = vector_base;
+typedef T value_type;
+typedef base::pointer pointer;
+using allocator_type = A;
+using size_type = int;
+using iterator = internal_iterator_type_template_we_dont_expect;
+using non_template_iterator = my_iterator;
+
+value_type& operator[](int index) { return elements[index]; }
+const value_type& at(int index) const { return elements[index]; }
+pointer data() { return &elements[0]; }
+allocator_type get_allocator() { return A(); }
+size_type size() const { return 10; }
+iterator begin() { return iterator(); }
+non_template_iterator end() { return non_template_iterator(); }
+
+T elements[10];
+  };
+
+  vector array;
+
+  auto $no_modifier[[by_value]] = array[3];
+  auto* $ptr_modifier[[ptr]] = &array[3];
+  auto& $ref_modifier[[ref]] = array[3];
+  auto& $at[[immutable]] = array.at(3);
+
+  auto $data[[data]] = array.data();
+  auto $allocator[[alloc]] = array.get_allocator();
+  auto $size[[size]] = array.size();
+  auto $begin[[begin]] = array.begin();
+  auto $end[[end]] = array.end();
+
+
+  // If the type alias is not of substituted template parameter type,
+  // do not show desugared type.
+  using VeryLongLongTypeName = my_iterator;
+  using Short = VeryLongLongTypeName;
+
+  auto $short_name[[my_value]] = Short();
+
+  // Same applies with templates.
+  template 
+  using basic_static_vector = vector;
+  template 
+  using static_vector = basic_static_vector>;
+
+  auto $vector_name[[vec]] = static_vector();
+  )cpp",
+  ExpectedHint{": int", "no_modifier"},
+  ExpectedHint{": int *", "ptr_modifier"},
+  ExpectedHint{": int &", "ref_modifier"},
+  ExpectedHint{": const int &", "at"}, ExpectedHint{": int *", "data"},
+  ExpectedHint{": allocator", "allocator"},
+  ExpectedHint{": size_type", "size"}, ExpectedHint{": iterator", "begin"},
+  ExpectedHint{": non_template_iterator", "end"},
+  ExpectedHint{": Short", "short_name"},
+  ExpectedHint{": static_vector", "vector_name"});
+}
+
 TEST(DesignatorHints, Basic) {
   assertDesignatorHints(R"cpp(
 struct S { int x, y, z; };
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -11,10 +11,12 @@
 #include "HeuristicResolver.h"
 #include "ParsedAST.h"
 #include "SourceCode.h"
+#include "clang/AST/ASTDiagnostic.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Type.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/ScopeExit.h"
@@ -190,6 +192,64 @@
   return Designators;
 }
 
+// Determines if any intermediate type in desugaring QualType QT is of
+// substituted template parameter type. Ignore pointer or reference wrappers.
+bool isSugaredTemplateParameter(QualType QT) {
+  static auto PeelWrappers = [](QualType QT) {
+// Neither `PointerType` nor `ReferenceType` is considered as sugared
+// type. Peel it.
+QualType Next;
+while (!(Next = QT->getPointeeType()).isNull())
+  QT = Next;
+return QT;
+  };
+  while (true) {
+QualType Desugared =
+PeelWrappers(QT->getLocallyUnqualifiedSingleStepDesugaredType());
+if (Desugared == QT)
+  break;
+if (Desugared->getAs())
+  return true;
+QT = Desugared;
+  }
+  return false;
+}
+
+// A simple wrapper for `clang::desugarForDiagnostic` that provides optional
+// semantic.
+std::optional desugar(ASTContext &AST, QualType QT) {
+  bool ShouldAKA;
+  auto Desugared = clang::desugarForDiagnostic(AST, QT, ShouldAKA);
+  if (!ShouldAKA)
+return std::nullopt;
+  return Desugared;
+}
+
+// Apply a series of heuristic methods to determine whether or not a QualType QT
+// is suitable for desugaring

[PATCH] D151785: [clangd] Desugar template parameter aliases in type hints

[Younan Zhang via Phabricator via cfe-commits]

zyounan updated this revision to Diff 530850.
zyounan marked an inline comment as done.
zyounan added a comment.

Final update


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151785/new/

https://reviews.llvm.org/D151785

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1417,6 +1417,82 @@
   ExpectedHint{": int", "h"}, ExpectedHint{": int", "i"});
 }
 
+TEST(TypeHints, SubstTemplateParameterAliases) {
+  assertTypeHints(
+  R"cpp(
+  template  struct allocator {};
+
+  template 
+  struct vector_base {
+using pointer = T*;
+  };
+
+  template 
+  struct internal_iterator_type_template_we_dont_expect {};
+
+  struct my_iterator {};
+
+  template >
+  struct vector : vector_base {
+using base = vector_base;
+typedef T value_type;
+typedef base::pointer pointer;
+using allocator_type = A;
+using size_type = int;
+using iterator = internal_iterator_type_template_we_dont_expect;
+using non_template_iterator = my_iterator;
+
+value_type& operator[](int index) { return elements[index]; }
+const value_type& at(int index) const { return elements[index]; }
+pointer data() { return &elements[0]; }
+allocator_type get_allocator() { return A(); }
+size_type size() const { return 10; }
+iterator begin() { return iterator(); }
+non_template_iterator end() { return non_template_iterator(); }
+
+T elements[10];
+  };
+
+  vector array;
+
+  auto $no_modifier[[by_value]] = array[3];
+  auto* $ptr_modifier[[ptr]] = &array[3];
+  auto& $ref_modifier[[ref]] = array[3];
+  auto& $at[[immutable]] = array.at(3);
+
+  auto $data[[data]] = array.data();
+  auto $allocator[[alloc]] = array.get_allocator();
+  auto $size[[size]] = array.size();
+  auto $begin[[begin]] = array.begin();
+  auto $end[[end]] = array.end();
+
+
+  // If the type alias is not of substituted template parameter type,
+  // do not show desugared type.
+  using VeryLongLongTypeName = my_iterator;
+  using Short = VeryLongLongTypeName;
+
+  auto $short_name[[my_value]] = Short();
+
+  // Same applies with templates.
+  template 
+  using basic_static_vector = vector;
+  template 
+  using static_vector = basic_static_vector>;
+
+  auto $vector_name[[vec]] = static_vector();
+  )cpp",
+  ExpectedHint{": int", "no_modifier"},
+  ExpectedHint{": int *", "ptr_modifier"},
+  ExpectedHint{": int &", "ref_modifier"},
+  ExpectedHint{": const int &", "at"}, ExpectedHint{": int *", "data"},
+  ExpectedHint{": allocator", "allocator"},
+  ExpectedHint{": size_type", "size"}, ExpectedHint{": iterator", "begin"},
+  ExpectedHint{": non_template_iterator", "end"},
+  ExpectedHint{": Short", "short_name"},
+  ExpectedHint{": static_vector", "vector_name"});
+}
+
 TEST(DesignatorHints, Basic) {
   assertDesignatorHints(R"cpp(
 struct S { int x, y, z; };
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -11,10 +11,12 @@
 #include "HeuristicResolver.h"
 #include "ParsedAST.h"
 #include "SourceCode.h"
+#include "clang/AST/ASTDiagnostic.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Type.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/ScopeExit.h"
@@ -190,6 +192,64 @@
   return Designators;
 }
 
+// Determines if any intermediate type in desugaring QualType QT is of
+// substituted template parameter type. Ignore pointer or reference wrappers.
+bool isSugaredTemplateParameter(QualType QT) {
+  static auto PeelWrappers = [](QualType QT) {
+// Neither `PointerType` nor `ReferenceType` is considered as sugared
+// type. Peel it.
+QualType Next;
+while (!(Next = QT->getPointeeType()).isNull())
+  QT = Next;
+return QT;
+  };
+  while (true) {
+QualType Desugared =
+PeelWrappers(QT->getLocallyUnqualifiedSingleStepDesugaredType());
+if (Desugared == QT)
+  break;
+if (Desugared->getAs())
+  return true;
+QT = Desugared;
+  }
+  return false;
+}
+
+// A simple wrapper for `clang::desugarForDiagnostic` that provides optional
+// semantic.
+std::optional desugar(ASTContext &AST, QualType QT) {
+  bool ShouldAKA;
+  auto Desugared = clang::desugarForDiagnostic(AST, QT, ShouldAKA);
+  if (!ShouldAKA)
+return std::nullopt;
+  return Desugared;
+}
+
+// Apply a series of heuristic methods to determine whether or not a QualType QT
+// is suitabl

[PATCH] D151785: [clangd] Desugar template parameter aliases in type hints

[Nathan Ridge via Phabricator via cfe-commits]

nridge accepted this revision.
nridge added a comment.
This revision is now accepted and ready to land.

Thanks, LGTM!




Comment at: clang-tools-extra/clangd/InlayHints.cpp:196
+// Determines if any intermediate type in desugaring QualType QT is of
+// substituted template parameter. Ignore pointer or reference wrappers.
+bool isSugaredTemplateParameter(QualType QT) {

nit: "of substituted template parameter" --> "a substituted template parmeter" 

(or "of substituted template parameter type")


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151785/new/

https://reviews.llvm.org/D151785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits