[clang-tools-extra] [clangd] Allow hover over 128-bit variable without crashing (PR #71415)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/71415
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Allow hover over 128-bit variable without crashing (PR #71415)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 approved this pull request.

Thank you! I didn't realize that `getActiveBits` was calculated by the count of 
leading zeros, even for negative numbers. This generally looks good modulo a 
question on the test.


https://github.com/llvm/llvm-project/pull/71415
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Allow hover over 128-bit variable without crashing (PR #71415)

[Younan Zhang via cfe-commits]

@@ -0,0 +1,29 @@
+# RUN: clangd -lit-test < %s | FileCheck %s

zyn0217 wrote:

Rather than creating a lit-test, could you please move it to the 
`HoverTests.cpp` under the unittests directory? That would be more 
straightforward, and I think we could add the case to `Hover.NoCrashAPInt64` 
(or feel free to rename that if you don't like the name.)

https://github.com/llvm/llvm-project/pull/71415
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Don't show inlay hints for __builtin_dump_struct (PR #71366)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/71366
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Don't show inlay hints for __builtin_dump_struct (PR #71366)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/71366
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Don't show inlay hints for __builtin_dump_struct (PR #71366)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

On second thought I think I should not kill all the expressions involving 
`PseudoObjectExpr`: I'm not sure if our ObjC / CUDA support does need that for 
presenting useful hints.

https://github.com/llvm/llvm-project/pull/71366
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Don't show inlay hints for __builtin_dump_struct (PR #71366)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 updated 
https://github.com/llvm/llvm-project/pull/71366

>From 4a878b63cbdd33833b998896120a992178438180 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Mon, 6 Nov 2023 16:50:02 +0800
Subject: [PATCH 1/3] [clangd] Don't show inlay hints for PseudoObjectExprs in
 C++

This closes https://github.com/clangd/clangd/issues/1813.

PseudoObjectExprs in C++ are currently not very interesting but probably
mess up inlay hints.
---
 clang-tools-extra/clangd/InlayHints.cpp   | 13 ++
 .../clangd/unittests/InlayHintTests.cpp   | 24 +++
 2 files changed, 37 insertions(+)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 3da047f95421385..867c70e5dbc80c6 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -589,6 +589,19 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
 return true;
   }
 
+  bool dataTraverseStmtPre(Stmt *S) {
+// Do not show inlay hints for PseudoObjectExprs. They're never
+// genuine user codes in C++.
+//
+// For example, __builtin_dump_struct would expand to a PseudoObjectExpr
+// that includes a couple of calls to a printf function. Printing parameter
+// names for that anyway would end up with duplicate parameter names 
(which,
+// however, got de-duplicated after visiting) for the printf function.
+if (AST.getLangOpts().CPlusPlus && isa(S))
+  return false;
+return true;
+  }
+
   bool VisitCallExpr(CallExpr *E) {
 if (!Cfg.InlayHints.Parameters)
   return true;
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp 
b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 20c1cdd985dbc01..2b6c27b17b537a9 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1724,6 +1724,30 @@ TEST(InlayHints, RestrictRange) {
   ElementsAre(labelIs(": int"), labelIs(": char")));
 }
 
+TEST(ParameterHints, PseudoObjectExpr) {
+  Annotations Code(R"cpp(
+struct S {
+  __declspec(property(get=GetX, put=PutX)) int x[];
+  int GetX(int y, int z) { return 42 + y; }
+  void PutX(int y) { x = y; } // Not `x = y: y`
+};
+
+int printf(const char *Format, ...);
+
+int main() {
+  S s;
+  __builtin_dump_struct(&s, printf); // Not `Format: 
__builtin_dump_struct()`
+  printf($Param[["Hello, %d"]], 42); // Normal calls are not affected.
+  return s.x[1][2]; // Not `x[y: 1][z: 2]`
+}
+  )cpp");
+  auto TU = TestTU::withCode(Code.code());
+  TU.ExtraArgs.push_back("-fms-extensions");
+  auto AST = TU.build();
+  EXPECT_THAT(inlayHints(AST, std::nullopt),
+  ElementsAre(HintMatcher(ExpectedHint{"Format: ", "Param"}, 
Code)));
+}
+
 TEST(ParameterHints, ArgPacksAndConstructors) {
   assertParameterHints(
   R"cpp(

>From 240b7f89e9bc9eb57f35f43be2f971b4fde76b46 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Mon, 6 Nov 2023 17:32:50 +0800
Subject: [PATCH 2/3] Don't make the patch C++-specific

---
 clang-tools-extra/clangd/InlayHints.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 867c70e5dbc80c6..1b54b570c1c5d4f 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -591,13 +591,13 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
 
   bool dataTraverseStmtPre(Stmt *S) {
 // Do not show inlay hints for PseudoObjectExprs. They're never
-// genuine user codes in C++.
+// genuine user codes.
 //
 // For example, __builtin_dump_struct would expand to a PseudoObjectExpr
 // that includes a couple of calls to a printf function. Printing parameter
 // names for that anyway would end up with duplicate parameter names 
(which,
 // however, got de-duplicated after visiting) for the printf function.
-if (AST.getLangOpts().CPlusPlus && isa(S))
+if (isa(S))
   return false;
 return true;
   }

>From 9de5c59af13188e9037612a5e73966d3ab9c576c Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Tue, 7 Nov 2023 14:22:07 +0800
Subject: [PATCH 3/3] Only apply the restriction to __builtin_dump_struct

---
 clang-tools-extra/clangd/InlayHints.cpp| 18 +-
 .../clangd/unittests/InlayHintTests.cpp|  9 ++---
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 1b54b570c1c5d4f..9897d1c1e6da209 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -590,15 +590,15 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
   }
 
   bool dataTraverseStmtPre(Stmt *S) {
-// Do not show inlay hints for PseudoObjectExprs. They're never
-// genuine user codes.
-//
- 

[clang-tools-extra] [clangd] Allow hover over 128-bit variable without crashing (PR #71415)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

Thanks! Please go ahead and merge it. :-)

https://github.com/llvm/llvm-project/pull/71415
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Correctly identify the next token after the completion point (PR #69153)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/69153
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Correctly identify the next token after the completion point (PR #69153)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 approved this pull request.

I'd like to approve the patch boldly since the findTokenAfterCompletionPoint is 
mostly duplicated from `Lexer::findNextToken` except for a difference that our 
version calls `Loc.getLocWithOffset(1)` rather than MeasureTokenLength() for 
the next token's location.

I have also tried another approach similar to Nathan's, in which I have 
constructed a lexer from the current location (i.e. the completion point) as 
well as the preprocessor. It looks as complicated as this way, and I don't 
think that is more optimal. (Or should I conclude that this is the best option?)

https://github.com/llvm/llvm-project/pull/69153
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Correctly identify the next token after the completion point (PR #69153)

[Younan Zhang via cfe-commits]

@@ -1491,6 +1491,46 @@ FuzzyFindRequest 
speculativeFuzzyFindRequestForCompletion(
   return CachedReq;
 }
 
+// This function is similar to Lexer::findNextToken(), but assumes
+// that the input SourceLocation is the completion point (which is
+// a case findNextToken() does not handle).
+std::optional
+findTokenAfterCompletionPoint(SourceLocation CompletionPoint,
+  const SourceManager &SM,
+  const LangOptions &LangOpts) {
+  SourceLocation Loc = CompletionPoint;

zyn0217 wrote:

I was about to comment that we should add an assertion here to reflect the 
contract. However, after going through the codes, it seems hard to tell the 
token kind under a source location without a lexer. :(

Also, do you think it's possible to make this function a static member of 
`Lexer` like `Lexer::findNextToken`? That way we might need a slight refactor 
to reuse some common logics.

https://github.com/llvm/llvm-project/pull/69153
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Correctly identify the next token after the completion point (PR #69153)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/69153
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Correctly identify the next token after the completion point (PR #69153)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/69153
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Resolve the dependent type from its single instantiation. Take 1 (PR #71279)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 updated 
https://github.com/llvm/llvm-project/pull/71279

>From d73a8e2ee683e6812c21cb1de7363b14565a96d1 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Sat, 4 Nov 2023 18:43:58 +0800
Subject: [PATCH 1/2] [clangd] Resolve the dependent type from its single
 instantiation. Take 1

This is an enhancement to the HeuristicResolver, trying to extract
the deduced type from the single instantiation for a template. This
partially addresses the point #1 from
https://github.com/clangd/clangd/issues/1768.

This patch doesn't tackle CXXUnresolvedConstructExpr or similarities
since I feel that is more arduous and would prefer to leave it for
my future work.
---
 .../clangd/HeuristicResolver.cpp  | 101 ++
 .../clangd/unittests/XRefsTests.cpp   |  48 +
 2 files changed, 149 insertions(+)

diff --git a/clang-tools-extra/clangd/HeuristicResolver.cpp 
b/clang-tools-extra/clangd/HeuristicResolver.cpp
index 3c147b6b582bf0b..d3dced9b325367a 100644
--- a/clang-tools-extra/clangd/HeuristicResolver.cpp
+++ b/clang-tools-extra/clangd/HeuristicResolver.cpp
@@ -7,10 +7,14 @@
 
//===--===//
 
 #include "HeuristicResolver.h"
+#include "AST.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
 
 namespace clang {
@@ -46,6 +50,98 @@ const Type *resolveDeclsToType(const std::vector &Decls,
   return nullptr;
 }
 
+// Visitor that helps to extract deduced type from instantiated entities.
+// This merely performs the source location comparison against each Decl
+// until it finds a Decl with the same location as the
+// dependent one. Its associated type will then be extracted.
+struct InstantiatedDeclVisitor : RecursiveASTVisitor {
+
+  InstantiatedDeclVisitor(NamedDecl *DependentDecl) : 
DependentDecl(DependentDecl) {}
+
+  bool shouldVisitTemplateInstantiations() const { return true; }
+
+  bool shouldVisitLambdaBody() const { return true; }
+
+  bool shouldVisitImplicitCode() const { return true; }
+
+  template 
+  bool onDeclVisited(D *MaybeInstantiated) {
+if (MaybeInstantiated->getDeclContext()->isDependentContext())
+  return true;
+auto *Dependent = dyn_cast(DependentDecl);
+if (!Dependent)
+  return true;
+auto LHS = MaybeInstantiated->getTypeSourceInfo(),
+ RHS = Dependent->getTypeSourceInfo();
+if (!LHS || !RHS)
+  return true;
+if (LHS->getTypeLoc().getSourceRange() !=
+RHS->getTypeLoc().getSourceRange())
+  return true;
+DeducedType = MaybeInstantiated->getType();
+return false;
+  }
+
+  bool VisitFieldDecl(FieldDecl *FD) {
+return onDeclVisited(FD);
+  }
+
+  bool VisitVarDecl(VarDecl *VD) {
+return onDeclVisited(VD);
+  }
+
+  NamedDecl *DependentDecl;
+  QualType DeducedType;
+};
+
+/// Attempt to resolve the dependent type from the surrounding context for 
which
+/// a single instantiation is available.
+const Type *
+resolveTypeFromInstantiatedTemplate(const CXXDependentScopeMemberExpr *Expr) {
+  if (Expr->isImplicitAccess())
+return nullptr;
+
+  auto *Base = Expr->getBase();
+  NamedDecl *ND = nullptr;
+  if (auto *CXXMember = dyn_cast(Base))
+ND = CXXMember->getMemberDecl();
+
+  if (auto *DRExpr = dyn_cast(Base))
+ND = DRExpr->getFoundDecl();
+
+  // FIXME: Handle CXXUnresolvedConstructExpr. This kind of type doesn't have
+  // available Decls to be matched against. Which inhibits the current 
heuristic
+  // from resolving expressions such as `T().fo^o()`, where T is a
+  // single-instantiated template parameter.
+  if (!ND)
+return nullptr;
+
+  NamedDecl *Instantiation = nullptr;
+
+  // Find out a single instantiation that we can start with. The enclosing
+  // context for the current Decl might not be a templated entity (e.g. a 
member
+  // function inside a class template), hence we shall walk up the decl
+  // contexts first.
+  for (auto *EnclosingContext = ND->getDeclContext(); EnclosingContext;
+   EnclosingContext = EnclosingContext->getParent()) {
+if (auto *ND = dyn_cast(EnclosingContext)) {
+  Instantiation = getOnlyInstantiation(ND);
+  if (Instantiation)
+break;
+}
+  }
+
+  if (!Instantiation)
+return nullptr;
+
+  // This will traverse down the instantiation entity, visit each Decl, and
+  // extract the deduced type for the undetermined Decl `ND`.
+  InstantiatedDeclVisitor Visitor(ND);
+  Visitor.TraverseDecl(Instantiation);
+
+  return Visitor.DeducedType.getTypePtrOrNull();
+}
+
 } // namespace
 
 // Helper function for HeuristicResolver::resolveDependentMember()
@@ -150,6 +246,11 @@ std::vector 
HeuristicResolver::resolveMemberExpr(
   if (ME->isArrow()) {
 BaseType = getPointeeType(BaseType);
   }
+
+  i

[clang-tools-extra] [clangd] Resolve the dependent type from its single instantiation. Take 1 (PR #71279)

[Younan Zhang via cfe-commits]

@@ -150,6 +246,11 @@ std::vector 
HeuristicResolver::resolveMemberExpr(
   if (ME->isArrow()) {
 BaseType = getPointeeType(BaseType);
   }
+
+  if (BaseType->isDependentType())

zyn0217 wrote:

I don't know if I should assume the non-nullity for `BaseType`: All of the 
construction points appear to create expressions with valid base types. Except 
for ASTImporter and ASTReader, where an empty expression may be created. But 
would we somehow end up resolving expressions created from these two places? I 
suppose we might encounter these inside a preamble, but I don't think this is 
where the heuristic resolver would work.

Anyway, I moved this statement after the non-null check. Thanks for spotting it!

https://github.com/llvm/llvm-project/pull/71279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Resolve the dependent type from its single instantiation. Take 1 (PR #71279)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 updated 
https://github.com/llvm/llvm-project/pull/71279

>From d73a8e2ee683e6812c21cb1de7363b14565a96d1 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Sat, 4 Nov 2023 18:43:58 +0800
Subject: [PATCH 1/3] [clangd] Resolve the dependent type from its single
 instantiation. Take 1

This is an enhancement to the HeuristicResolver, trying to extract
the deduced type from the single instantiation for a template. This
partially addresses the point #1 from
https://github.com/clangd/clangd/issues/1768.

This patch doesn't tackle CXXUnresolvedConstructExpr or similarities
since I feel that is more arduous and would prefer to leave it for
my future work.
---
 .../clangd/HeuristicResolver.cpp  | 101 ++
 .../clangd/unittests/XRefsTests.cpp   |  48 +
 2 files changed, 149 insertions(+)

diff --git a/clang-tools-extra/clangd/HeuristicResolver.cpp 
b/clang-tools-extra/clangd/HeuristicResolver.cpp
index 3c147b6b582bf0b..d3dced9b325367a 100644
--- a/clang-tools-extra/clangd/HeuristicResolver.cpp
+++ b/clang-tools-extra/clangd/HeuristicResolver.cpp
@@ -7,10 +7,14 @@
 
//===--===//
 
 #include "HeuristicResolver.h"
+#include "AST.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
 
 namespace clang {
@@ -46,6 +50,98 @@ const Type *resolveDeclsToType(const std::vector &Decls,
   return nullptr;
 }
 
+// Visitor that helps to extract deduced type from instantiated entities.
+// This merely performs the source location comparison against each Decl
+// until it finds a Decl with the same location as the
+// dependent one. Its associated type will then be extracted.
+struct InstantiatedDeclVisitor : RecursiveASTVisitor {
+
+  InstantiatedDeclVisitor(NamedDecl *DependentDecl) : 
DependentDecl(DependentDecl) {}
+
+  bool shouldVisitTemplateInstantiations() const { return true; }
+
+  bool shouldVisitLambdaBody() const { return true; }
+
+  bool shouldVisitImplicitCode() const { return true; }
+
+  template 
+  bool onDeclVisited(D *MaybeInstantiated) {
+if (MaybeInstantiated->getDeclContext()->isDependentContext())
+  return true;
+auto *Dependent = dyn_cast(DependentDecl);
+if (!Dependent)
+  return true;
+auto LHS = MaybeInstantiated->getTypeSourceInfo(),
+ RHS = Dependent->getTypeSourceInfo();
+if (!LHS || !RHS)
+  return true;
+if (LHS->getTypeLoc().getSourceRange() !=
+RHS->getTypeLoc().getSourceRange())
+  return true;
+DeducedType = MaybeInstantiated->getType();
+return false;
+  }
+
+  bool VisitFieldDecl(FieldDecl *FD) {
+return onDeclVisited(FD);
+  }
+
+  bool VisitVarDecl(VarDecl *VD) {
+return onDeclVisited(VD);
+  }
+
+  NamedDecl *DependentDecl;
+  QualType DeducedType;
+};
+
+/// Attempt to resolve the dependent type from the surrounding context for 
which
+/// a single instantiation is available.
+const Type *
+resolveTypeFromInstantiatedTemplate(const CXXDependentScopeMemberExpr *Expr) {
+  if (Expr->isImplicitAccess())
+return nullptr;
+
+  auto *Base = Expr->getBase();
+  NamedDecl *ND = nullptr;
+  if (auto *CXXMember = dyn_cast(Base))
+ND = CXXMember->getMemberDecl();
+
+  if (auto *DRExpr = dyn_cast(Base))
+ND = DRExpr->getFoundDecl();
+
+  // FIXME: Handle CXXUnresolvedConstructExpr. This kind of type doesn't have
+  // available Decls to be matched against. Which inhibits the current 
heuristic
+  // from resolving expressions such as `T().fo^o()`, where T is a
+  // single-instantiated template parameter.
+  if (!ND)
+return nullptr;
+
+  NamedDecl *Instantiation = nullptr;
+
+  // Find out a single instantiation that we can start with. The enclosing
+  // context for the current Decl might not be a templated entity (e.g. a 
member
+  // function inside a class template), hence we shall walk up the decl
+  // contexts first.
+  for (auto *EnclosingContext = ND->getDeclContext(); EnclosingContext;
+   EnclosingContext = EnclosingContext->getParent()) {
+if (auto *ND = dyn_cast(EnclosingContext)) {
+  Instantiation = getOnlyInstantiation(ND);
+  if (Instantiation)
+break;
+}
+  }
+
+  if (!Instantiation)
+return nullptr;
+
+  // This will traverse down the instantiation entity, visit each Decl, and
+  // extract the deduced type for the undetermined Decl `ND`.
+  InstantiatedDeclVisitor Visitor(ND);
+  Visitor.TraverseDecl(Instantiation);
+
+  return Visitor.DeducedType.getTypePtrOrNull();
+}
+
 } // namespace
 
 // Helper function for HeuristicResolver::resolveDependentMember()
@@ -150,6 +246,11 @@ std::vector 
HeuristicResolver::resolveMemberExpr(
   if (ME->isArrow()) {
 BaseType = getPointeeType(BaseType);
   }
+
+  i

[clang] [clang][Sema] Avoid non-empty unexpanded pack assertion for FunctionParmPackExpr (PR #69224)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 closed 
https://github.com/llvm/llvm-project/pull/69224
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Resolve the dependent type from its single instantiation. Take 1 (PR #71279)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

> If so, get its TemplateTypeParmDecl, find the template whose parameter it is, 
> and see if the template has a getOnlyInstantiation.

Thanks for the suggestion! Admittedly, it looks neat and terse. However, I 
suspect it won't work with the present `TemplateTypeParm{Type,Decl}` models. 
`TemplateTypeParmDecl` per se is not tied to any `CXXRecordDecl` nor 
`FunctionDecl` that  the template parameter is declaring with. (For the most 
seemingly-relevant part, i.e. the `DeclContext` it records, is the scope that 
the template defines.)

> If so, find the argument type substituted for this template parameter, and 
> replace the template parameter with this type.

I was thinking this way before getting into the details :-). Soon I realized 
that we had to tackle synthesized types e.g.
```cpp
template  typename C, typename D, int E>
void work() {
  C complicated_type;
  // ^ Shall we perform the type substitution once again? ;)
}
```




https://github.com/llvm/llvm-project/pull/71279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Don't show inlay hints for __builtin_dump_struct (PR #71366)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

> Is it not possible to tell based on the `CallExpr` only (rather than checking 
> an ancestor of it during traversal of the tree) that "this `CallExpr` is not 
> explicitly written in the source"?

I'm not quite sure - Looking into the construction process for a 
`__builtin_dump_struct` expression,
https://github.com/llvm/llvm-project/blob/4d5a8ccf25a6c43930c332e75ecead287778af6b/clang/lib/Sema/SemaChecking.cpp#L482-L509

the part that makes the call unordinary might be the nullish scope. I don't 
know if that implies "the expression is not written by users"; even so, 
extracting the scope of a call expression then performing the check doesn't 
look much more straightforward than this way.



https://github.com/llvm/llvm-project/pull/71366
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Check nullness of captured type before use (PR #72230)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

Could you please explain why the `CaptureType` might be null at the time when 
exercising `tryCaptureVariable`? I'm worried that doing so would hide a more 
significant issue.

https://github.com/llvm/llvm-project/pull/72230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Reject incomplete type arguments for __builtin_dump_struct (PR #72749)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/72749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [clang-tools-extra] [clang] Reject incomplete type arguments for __builtin_dump_struct (PR #72749)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 updated 
https://github.com/llvm/llvm-project/pull/72749

>From d23305db7faba1ed1464aeee6d9e0f2ee1994226 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Mon, 6 Nov 2023 16:50:02 +0800
Subject: [PATCH 1/2] [clang] Reject incomplete type arguments for
 __builtin_dump_struct

We used to assume that the CXXRecordDecl passed to the 1st argument
always had a definition. This is not true since a pointer to an
incomplete type was not excluded.
---
 clang/docs/LanguageExtensions.rst  | 2 +-
 clang/docs/ReleaseNotes.rst| 3 +++
 clang/lib/Sema/SemaChecking.cpp| 5 +
 clang/test/SemaCXX/builtin-dump-struct.cpp | 4 
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 30e288f986782..e80a096357b1d 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -2809,7 +2809,7 @@ Example output:
 
 The ``__builtin_dump_struct`` function is used to print the fields of a simple
 structure and their values for debugging purposes. The first argument of the
-builtin should be a pointer to the struct to dump. The second argument ``f``
+builtin should be a pointer to a complete record type to dump. The second 
argument ``f``
 should be some callable expression, and can be a function object or an overload
 set. The builtin calls ``f``, passing any further arguments ``args...``
 followed by a ``printf``-compatible format string and the corresponding
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 74358219ba9fb..f2903c1684dcb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -691,6 +691,9 @@ Bug Fixes to C++ Support
   Fixes:
   (`#68769 `_)
 
+- Clang now rejects incomplete types for ``__builtin_dump_struct``. Fixes:
+  (`#63506 `_)
+
 - Clang now defers the instantiation of explicit specifier until constraint 
checking
   completes (except deduction guides). Fixes:
   (`#59827 `_)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ae588db02bbe7..20ff3ce722da8 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -713,6 +713,11 @@ static ExprResult SemaBuiltinDumpStruct(Sema &S, CallExpr 
*TheCall) {
 return ExprError();
   }
   const RecordDecl *RD = PtrArgType->getPointeeType()->getAsRecordDecl();
+  if (!RD->isCompleteDefinition()) {
+S.Diag(PtrArgResult.get()->getBeginLoc(), diag::err_incomplete_type)
+<< PtrArgType->getPointeeType();
+return ExprError();
+  }
 
   // Second argument is a callable, but we can't fully validate it until we try
   // calling it.
diff --git a/clang/test/SemaCXX/builtin-dump-struct.cpp 
b/clang/test/SemaCXX/builtin-dump-struct.cpp
index b3d2a2d808ce2..477bbcf07a41f 100644
--- a/clang/test/SemaCXX/builtin-dump-struct.cpp
+++ b/clang/test/SemaCXX/builtin-dump-struct.cpp
@@ -149,7 +149,10 @@ B {
 }
 )"[1]);
 
+class Incomplete;
+
 void errors(B b) {
+  ConstexprString cs;
   __builtin_dump_struct(); // expected-error {{too few arguments to function 
call, expected 2, have 0}}
   __builtin_dump_struct(1); // expected-error {{too few arguments to function 
call, expected 2, have 1}}
   __builtin_dump_struct(1, 2); // expected-error {{expected pointer to struct 
as 1st argument to '__builtin_dump_struct', found 'int'}}
@@ -157,6 +160,7 @@ void errors(B b) {
   __builtin_dump_struct(&b, Format, 0); // expected-error {{no matching 
function for call to 'Format'}}
 // expected-note@-1 {{in call to 
printing function with arguments '(0, "%s", "B")' while dumping struct}}
 // expected-note@#Format {{no known 
conversion from 'int' to 'ConstexprString &' for 1st argument}}
+  __builtin_dump_struct((Incomplete *)nullptr, Format, cs); // expected-error 
{{incomplete type 'Incomplete' where a complete type is required}}
 }
 #endif
 

>From e0bbbd2e5518a98936e1b6f51e8fff66018a7183 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Sun, 19 Nov 2023 23:38:50 +0800
Subject: [PATCH 2/2] Use Sema::RequireCompleteType.

---
 clang/lib/Sema/SemaChecking.cpp| 12 ++--
 clang/test/SemaCXX/builtin-dump-struct.cpp | 10 +-
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 20ff3ce722da8..9f90585b3b7d8 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -712,13 +712,13 @@ static ExprResult SemaBuiltinDumpStruct(Sema &S, CallExpr 
*TheCall) {
 << 1 << TheCall->getDirectCallee() << PtrArgType;
 return ExprError();
   }
-  const RecordDecl *RD = PtrArgType->getPointeeType()->getAsRecordDecl();
-  if (!RD->isCompleteDefi

[clang] [llvm] [clang-tools-extra] [clang] Reject incomplete type arguments for __builtin_dump_struct (PR #72749)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

Thanks! Added the link.

https://github.com/llvm/llvm-project/pull/72749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [clang-tools-extra] [clang] Reject incomplete type arguments for __builtin_dump_struct (PR #72749)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 closed 
https://github.com/llvm/llvm-project/pull/72749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Resolve the dependent type from its single instantiation. Take 1 (PR #71279)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

@HighCommander4 While at the `HeuristicResolver`, I think we may have a bug 
inside `HighlightingsBuilder` defined in `SemanticHighlighting.cpp`.

https://github.com/llvm/llvm-project/blob/6ed9a81f7ebd23f125867dd270785dd0e63043c6/clang-tools-extra/clangd/SemanticHighlighting.cpp#L592

I didn't see any other lines initializing this member. so we're always yielding 
`nullptr` from `H.getResolver` calls?

https://github.com/llvm/llvm-project/pull/71279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Initialize HighlightingsBuilder::Resolver (PR #74971)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 approved this pull request.

Thank you! LGTM modulo one nit: could you please remove the nullptr member 
initializer for `Resolver`? That looks unnecessary now.

https://github.com/llvm/llvm-project/pull/74971
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Correctly implement CWG 2672 (PR #75001)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 created 
https://github.com/llvm/llvm-project/pull/75001

This is a follow-up patch for [D156993](https://reviews.llvm.org/D156993), that 
marks only the lambda body as non-immediate context.

Fixes https://github.com/llvm/llvm-project/issues/71684

>From 8681b3c9f5e19b6ae977321d5d4154113273c2a0 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Sun, 12 Nov 2023 13:21:03 +0800
Subject: [PATCH] [clang] Correctly implement CWG 2672

This is a follow-up patch for [D156993](https://reviews.llvm.org/D156993),
that marks only the lambda body as non-immediate context.

Fixes https://github.com/llvm/llvm-project/issues/71684
---
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 11 +---
 clang/lib/Sema/TreeTransform.h|  7 +
 clang/test/CXX/drs/dr26xx.cpp |  2 +-
 .../expr.prim.lambda/default-arguments.cpp|  6 ++--
 .../expr.prim/expr.prim.lambda/p11-1y.cpp |  2 --
 .../expr/expr.prim/expr.prim.lambda/p23.cpp   |  1 -
 .../expr/expr.prim/expr.prim.lambda/p4.cpp|  3 +-
 clang/test/CXX/temp/temp.deduct/p9.cpp|  8 ++
 clang/test/SemaCXX/cxx1y-init-captures.cpp|  8 +++---
 clang/test/SemaCXX/cxx1z-lambda-star-this.cpp |  4 +--
 clang/test/SemaCXX/lambda-expressions.cpp |  4 +--
 clang/test/SemaCXX/lambda-pack-expansion.cpp  |  1 -
 clang/test/SemaCXX/vartemplate-lambda.cpp |  1 -
 .../SemaCXX/warn-unused-lambda-capture.cpp| 28 +--
 .../SemaTemplate/instantiate-local-class.cpp  |  4 +--
 15 files changed, 37 insertions(+), 53 deletions(-)

diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp 
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index df6b40999e645c..88bd44f7d6934d 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -35,7 +35,6 @@
 #include "clang/Sema/Template.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/TemplateInstCallback.h"
-#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -1142,8 +1141,7 @@ std::optional 
Sema::isSFINAEContext() const {
 case CodeSynthesisContext::DeducedTemplateArgumentSubstitution:
   // We're either substituting explicitly-specified template arguments,
   // deduced template arguments. SFINAE applies unless we are in a lambda
-  // expression, see [temp.deduct]p9.
-  [[fallthrough]];
+  // body, see [temp.deduct]p9.
 case CodeSynthesisContext::ConstraintSubstitution:
 case CodeSynthesisContext::RequirementInstantiation:
 case CodeSynthesisContext::RequirementParameterInstantiation:
@@ -1444,13 +1442,6 @@ namespace {
   LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true);
   Sema::ConstraintEvalRAII RAII(*this);
 
-  Sema::CodeSynthesisContext C;
-  C.Kind = clang::Sema::CodeSynthesisContext::LambdaExpressionSubstitution;
-  C.PointOfInstantiation = E->getBeginLoc();
-  SemaRef.pushCodeSynthesisContext(C);
-  auto PopCtx =
-  llvm::make_scope_exit([this] { SemaRef.popCodeSynthesisContext(); });
-
   ExprResult Result = inherited::TransformLambdaExpr(E);
   if (Result.isInvalid())
 return Result;
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 1ad843d0bf4e0c..55e5c3c9dedc56 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -13648,10 +13648,17 @@ 
TreeTransform::TransformLambdaExpr(LambdaExpr *E) {
   getSema().PushExpressionEvaluationContext(
   Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
 
+  Sema::CodeSynthesisContext C;
+  C.Kind = clang::Sema::CodeSynthesisContext::LambdaExpressionSubstitution;
+  C.PointOfInstantiation = E->getBody()->getBeginLoc();
+  getSema().pushCodeSynthesisContext(C);
+
   // Instantiate the body of the lambda expression.
   StmtResult Body =
   Invalid ? StmtError() : getDerived().TransformLambdaBody(E, 
E->getBody());
 
+  getSema().popCodeSynthesisContext();
+
   // ActOnLambda* will pop the function scope for us.
   FuncScopeCleanup.disable();
 
diff --git a/clang/test/CXX/drs/dr26xx.cpp b/clang/test/CXX/drs/dr26xx.cpp
index dd4bb1ff6ae2e1..8a22dbeb98a3d5 100644
--- a/clang/test/CXX/drs/dr26xx.cpp
+++ b/clang/test/CXX/drs/dr26xx.cpp
@@ -211,7 +211,7 @@ void f(...);
 
 template 
 void bar(T) requires requires {
-   decltype([]() -> T {})::foo();
+   []() -> decltype(T::foo()) {};
 };
 void bar(...);
 
diff --git 
a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/default-arguments.cpp 
b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/default-arguments.cpp
index c5d08ec404a7c3..72265d77700aaf 100644
--- a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/default-arguments.cpp
+++ b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/default-arguments.cpp
@@ -35,8 +35,7 @@ struct NoDefaultCtor {
 template
 void defargs_in_template_unused(T t) {
   auto l1 = [](const T& value

[clang] [clang] Correctly implement CWG 2672 (PR #75001)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

Note I'm not adding an extra release note since the previous implementation has 
done that and hasn't been cherry-picked or released yet.

https://github.com/llvm/llvm-project/pull/75001
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Don't show inlay hints for __builtin_dump_struct (PR #71366)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

> Out of curiosity, I dialled into today's LLVM office hours and asked Aaron 
> Ballman about this

Thank you so much for making the effort on this issue. :)

> ...just change the code for building the invented CallExprs to give them an 
> invalid SourceLocation

Interesting idea and I'll explore it.


https://github.com/llvm/llvm-project/pull/71366
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Reject incomplete type arguments for __builtin_dump_struct (PR #72749)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 created 
https://github.com/llvm/llvm-project/pull/72749

We used to assume that the CXXRecordDecl passed to the 1st argument always had 
a definition. This is not true since a pointer to an incomplete type was not 
excluded.

>From d23305db7faba1ed1464aeee6d9e0f2ee1994226 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Mon, 6 Nov 2023 16:50:02 +0800
Subject: [PATCH] [clang] Reject incomplete type arguments for
 __builtin_dump_struct

We used to assume that the CXXRecordDecl passed to the 1st argument
always had a definition. This is not true since a pointer to an
incomplete type was not excluded.
---
 clang/docs/LanguageExtensions.rst  | 2 +-
 clang/docs/ReleaseNotes.rst| 3 +++
 clang/lib/Sema/SemaChecking.cpp| 5 +
 clang/test/SemaCXX/builtin-dump-struct.cpp | 4 
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 30e288f986782fd..e80a096357b1deb 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -2809,7 +2809,7 @@ Example output:
 
 The ``__builtin_dump_struct`` function is used to print the fields of a simple
 structure and their values for debugging purposes. The first argument of the
-builtin should be a pointer to the struct to dump. The second argument ``f``
+builtin should be a pointer to a complete record type to dump. The second 
argument ``f``
 should be some callable expression, and can be a function object or an overload
 set. The builtin calls ``f``, passing any further arguments ``args...``
 followed by a ``printf``-compatible format string and the corresponding
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 74358219ba9fb22..f2903c1684dcb60 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -691,6 +691,9 @@ Bug Fixes to C++ Support
   Fixes:
   (`#68769 `_)
 
+- Clang now rejects incomplete types for ``__builtin_dump_struct``. Fixes:
+  (`#63506 `_)
+
 - Clang now defers the instantiation of explicit specifier until constraint 
checking
   completes (except deduction guides). Fixes:
   (`#59827 `_)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ae588db02bbe722..20ff3ce722da80a 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -713,6 +713,11 @@ static ExprResult SemaBuiltinDumpStruct(Sema &S, CallExpr 
*TheCall) {
 return ExprError();
   }
   const RecordDecl *RD = PtrArgType->getPointeeType()->getAsRecordDecl();
+  if (!RD->isCompleteDefinition()) {
+S.Diag(PtrArgResult.get()->getBeginLoc(), diag::err_incomplete_type)
+<< PtrArgType->getPointeeType();
+return ExprError();
+  }
 
   // Second argument is a callable, but we can't fully validate it until we try
   // calling it.
diff --git a/clang/test/SemaCXX/builtin-dump-struct.cpp 
b/clang/test/SemaCXX/builtin-dump-struct.cpp
index b3d2a2d808ce267..477bbcf07a41fa4 100644
--- a/clang/test/SemaCXX/builtin-dump-struct.cpp
+++ b/clang/test/SemaCXX/builtin-dump-struct.cpp
@@ -149,7 +149,10 @@ B {
 }
 )"[1]);
 
+class Incomplete;
+
 void errors(B b) {
+  ConstexprString cs;
   __builtin_dump_struct(); // expected-error {{too few arguments to function 
call, expected 2, have 0}}
   __builtin_dump_struct(1); // expected-error {{too few arguments to function 
call, expected 2, have 1}}
   __builtin_dump_struct(1, 2); // expected-error {{expected pointer to struct 
as 1st argument to '__builtin_dump_struct', found 'int'}}
@@ -157,6 +160,7 @@ void errors(B b) {
   __builtin_dump_struct(&b, Format, 0); // expected-error {{no matching 
function for call to 'Format'}}
 // expected-note@-1 {{in call to 
printing function with arguments '(0, "%s", "B")' while dumping struct}}
 // expected-note@#Format {{no known 
conversion from 'int' to 'ConstexprString &' for 1st argument}}
+  __builtin_dump_struct((Incomplete *)nullptr, Format, cs); // expected-error 
{{incomplete type 'Incomplete' where a complete type is required}}
 }
 #endif
 

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

[clang] [clang-tools-extra] [clang] Turn invented Exprs' source locations in __builtin_dump_struct to empty (PR #72750)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 created 
https://github.com/llvm/llvm-project/pull/72750

This reflects the comment in 
https://github.com/llvm/llvm-project/pull/71366#issuecomment-1817271492.

As that PR suggests, the invented CallExpr's source location previously pointed 
to the beginning of the `__builtin_dump_struct`. These spurious AST nodes 
confused clangd while displaying parameter inlay hints.

This patch takes another approach to address the same issue, by turning the 
location for each _argument_ within an invented call to printf to empty, 
(maybe) at the risk of breaking the invariant for CallExpr -- The source 
location for an argument could be invalid from now on.

>From cf12f37b477e53f60f22bbdd23eb44a1d0c9b91f Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Mon, 6 Nov 2023 16:50:02 +0800
Subject: [PATCH] [clang] Turn invented Exprs' source locations in
 __builtin_dump_struct to empty

This reflects the comment in
https://github.com/llvm/llvm-project/pull/71366#issuecomment-1817271492.

As that PR suggests, the invented CallExpr's source location previously
pointed to the beginning of the __builtin_dump_struct. These spurious
AST nodes confused clangd while displaying parameter inlay hints.

This patch takes another approach to address the same issue, by turning
the location for each _argument_ within an invented call to printf to empty,
(maybe) at the risk of breaking the invariant for CallExpr -- The source
location for an argument could be invalid from now on.
---
 .../clangd/unittests/InlayHintTests.cpp   | 27 +++
 clang/lib/Sema/SemaChecking.cpp   |  6 ++---
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp 
b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 20c1cdd985dbc01..47af261fad850e8 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1724,6 +1724,33 @@ TEST(InlayHints, RestrictRange) {
   ElementsAre(labelIs(": int"), labelIs(": char")));
 }
 
+TEST(ParameterHints, PseudoObjectExpr) {
+  Annotations Code(R"cpp(
+struct S {
+  __declspec(property(get=GetX, put=PutX)) int x[];
+  int GetX(int y, int z) { return 42 + y; }
+  void PutX(int y) { x = $one[[y]]; } // FIXME: Undesired `x = y: y` for 
this ill-formed expression.
+};
+
+int printf(const char *Format, ...);
+
+int main() {
+  S s;
+  __builtin_dump_struct(&s, printf); // Not `Format: 
__builtin_dump_struct()`
+  printf($Param[["Hello, %d"]], 42); // Normal calls are not affected.
+  return s.x[ $two[[1]] ][ $three[[2]] ]; // `x[y: 1][z: 2]`
+}
+  )cpp");
+  auto TU = TestTU::withCode(Code.code());
+  TU.ExtraArgs.push_back("-fms-extensions");
+  auto AST = TU.build();
+  EXPECT_THAT(inlayHints(AST, std::nullopt),
+  ElementsAre(HintMatcher(ExpectedHint{"y: ", "one"}, Code),
+  HintMatcher(ExpectedHint{"Format: ", "Param"}, Code),
+  HintMatcher(ExpectedHint{"y: ", "two"}, Code),
+  HintMatcher(ExpectedHint{"z: ", "three"}, Code)));
+}
+
 TEST(ParameterHints, ArgPacksAndConstructors) {
   assertParameterHints(
   R"cpp(
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ae588db02bbe722..5d348bd712dd104 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -454,13 +454,13 @@ namespace {
 struct BuiltinDumpStructGenerator {
   Sema &S;
   CallExpr *TheCall;
-  SourceLocation Loc = TheCall->getBeginLoc();
+  SourceLocation Loc;
   SmallVector Actions;
   DiagnosticErrorTrap ErrorTracker;
   PrintingPolicy Policy;
 
   BuiltinDumpStructGenerator(Sema &S, CallExpr *TheCall)
-  : S(S), TheCall(TheCall), ErrorTracker(S.getDiagnostics()),
+  : S(S), TheCall(TheCall), Loc(), ErrorTracker(S.getDiagnostics()),
 Policy(S.Context.getPrintingPolicy()) {
 Policy.AnonymousTagLocations = false;
   }
@@ -491,7 +491,7 @@ struct BuiltinDumpStructGenerator {
 // Register a note to explain why we're performing the call.
 Sema::CodeSynthesisContext Ctx;
 Ctx.Kind = Sema::CodeSynthesisContext::BuildingBuiltinDumpStructCall;
-Ctx.PointOfInstantiation = Loc;
+Ctx.PointOfInstantiation = TheCall->getBeginLoc();
 Ctx.CallArgs = Args.data();
 Ctx.NumCallArgs = Args.size();
 S.pushCodeSynthesisContext(Ctx);

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

[clang] [clang-tools-extra] [clang] Turn invented Exprs' source locations in __builtin_dump_struct to empty (PR #72750)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

> The CI run shows the test 
> [Sema/builtin-dump-struct.c](https://searchfox.org/llvm/source/clang/test/Sema/builtin-dump-struct.c)
>  is failing.

Oops, I didn't even realize there's such a test for C. I had only run the 
Sema/builtin-dump-struct.cpp locally and it passed. :(

> I guess this means the approach of using an invalid SourceLocation doesn't 
> work after all?
 
The answer seems no to me. At least we have to preserve these source locations 
at the Sema level to produce the diagnoses. As to the other approach involving 
"virtual buffers", I think that requires a more drastic change. I.e. moving the 
printf function generation to the tokenization level.

Closing this PR now and sorry for the churn!

https://github.com/llvm/llvm-project/pull/72750
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang] [clang] Turn invented Exprs' source locations in __builtin_dump_struct to empty (PR #72750)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 closed 
https://github.com/llvm/llvm-project/pull/72750
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Reject incomplete type arguments for __builtin_dump_struct (PR #72749)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 updated 
https://github.com/llvm/llvm-project/pull/72749

>From d23305db7faba1ed1464aeee6d9e0f2ee1994226 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Mon, 6 Nov 2023 16:50:02 +0800
Subject: [PATCH 1/2] [clang] Reject incomplete type arguments for
 __builtin_dump_struct

We used to assume that the CXXRecordDecl passed to the 1st argument
always had a definition. This is not true since a pointer to an
incomplete type was not excluded.
---
 clang/docs/LanguageExtensions.rst  | 2 +-
 clang/docs/ReleaseNotes.rst| 3 +++
 clang/lib/Sema/SemaChecking.cpp| 5 +
 clang/test/SemaCXX/builtin-dump-struct.cpp | 4 
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 30e288f986782fd..e80a096357b1deb 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -2809,7 +2809,7 @@ Example output:
 
 The ``__builtin_dump_struct`` function is used to print the fields of a simple
 structure and their values for debugging purposes. The first argument of the
-builtin should be a pointer to the struct to dump. The second argument ``f``
+builtin should be a pointer to a complete record type to dump. The second 
argument ``f``
 should be some callable expression, and can be a function object or an overload
 set. The builtin calls ``f``, passing any further arguments ``args...``
 followed by a ``printf``-compatible format string and the corresponding
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 74358219ba9fb22..f2903c1684dcb60 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -691,6 +691,9 @@ Bug Fixes to C++ Support
   Fixes:
   (`#68769 `_)
 
+- Clang now rejects incomplete types for ``__builtin_dump_struct``. Fixes:
+  (`#63506 `_)
+
 - Clang now defers the instantiation of explicit specifier until constraint 
checking
   completes (except deduction guides). Fixes:
   (`#59827 `_)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ae588db02bbe722..20ff3ce722da80a 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -713,6 +713,11 @@ static ExprResult SemaBuiltinDumpStruct(Sema &S, CallExpr 
*TheCall) {
 return ExprError();
   }
   const RecordDecl *RD = PtrArgType->getPointeeType()->getAsRecordDecl();
+  if (!RD->isCompleteDefinition()) {
+S.Diag(PtrArgResult.get()->getBeginLoc(), diag::err_incomplete_type)
+<< PtrArgType->getPointeeType();
+return ExprError();
+  }
 
   // Second argument is a callable, but we can't fully validate it until we try
   // calling it.
diff --git a/clang/test/SemaCXX/builtin-dump-struct.cpp 
b/clang/test/SemaCXX/builtin-dump-struct.cpp
index b3d2a2d808ce267..477bbcf07a41fa4 100644
--- a/clang/test/SemaCXX/builtin-dump-struct.cpp
+++ b/clang/test/SemaCXX/builtin-dump-struct.cpp
@@ -149,7 +149,10 @@ B {
 }
 )"[1]);
 
+class Incomplete;
+
 void errors(B b) {
+  ConstexprString cs;
   __builtin_dump_struct(); // expected-error {{too few arguments to function 
call, expected 2, have 0}}
   __builtin_dump_struct(1); // expected-error {{too few arguments to function 
call, expected 2, have 1}}
   __builtin_dump_struct(1, 2); // expected-error {{expected pointer to struct 
as 1st argument to '__builtin_dump_struct', found 'int'}}
@@ -157,6 +160,7 @@ void errors(B b) {
   __builtin_dump_struct(&b, Format, 0); // expected-error {{no matching 
function for call to 'Format'}}
 // expected-note@-1 {{in call to 
printing function with arguments '(0, "%s", "B")' while dumping struct}}
 // expected-note@#Format {{no known 
conversion from 'int' to 'ConstexprString &' for 1st argument}}
+  __builtin_dump_struct((Incomplete *)nullptr, Format, cs); // expected-error 
{{incomplete type 'Incomplete' where a complete type is required}}
 }
 #endif
 

>From e0bbbd2e5518a98936e1b6f51e8fff66018a7183 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Sun, 19 Nov 2023 23:38:50 +0800
Subject: [PATCH 2/2] Use Sema::RequireCompleteType.

---
 clang/lib/Sema/SemaChecking.cpp| 12 ++--
 clang/test/SemaCXX/builtin-dump-struct.cpp | 10 +-
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 20ff3ce722da80a..9f90585b3b7d8cb 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -712,13 +712,13 @@ static ExprResult SemaBuiltinDumpStruct(Sema &S, CallExpr 
*TheCall) {
 << 1 << TheCall->getDirectCallee() << PtrArgType;
 return ExprError();
   }
-  const RecordDecl *RD = PtrArgType->getPointeeType()->getAsRecordDecl();
-  if 

[clang] [clang] Reject incomplete type arguments for __builtin_dump_struct (PR #72749)

[Younan Zhang via cfe-commits]

@@ -713,6 +713,11 @@ static ExprResult SemaBuiltinDumpStruct(Sema &S, CallExpr 
*TheCall) {
 return ExprError();
   }
   const RecordDecl *RD = PtrArgType->getPointeeType()->getAsRecordDecl();
+  if (!RD->isCompleteDefinition()) {

zyn0217 wrote:

Thank you! Added the template case to the test.

https://github.com/llvm/llvm-project/pull/72749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Don't show inlay hints for __builtin_dump_struct (PR #71366)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 updated 
https://github.com/llvm/llvm-project/pull/71366

>From 808c141c34218dd542b00149216adc061567dd31 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Mon, 6 Nov 2023 16:50:02 +0800
Subject: [PATCH 1/4] [clangd] Don't show inlay hints for PseudoObjectExprs in
 C++

This closes https://github.com/clangd/clangd/issues/1813.

PseudoObjectExprs in C++ are currently not very interesting but probably
mess up inlay hints.
---
 clang-tools-extra/clangd/InlayHints.cpp   | 13 ++
 .../clangd/unittests/InlayHintTests.cpp   | 24 +++
 2 files changed, 37 insertions(+)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 3da047f954213856..867c70e5dbc80c65 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -589,6 +589,19 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
 return true;
   }
 
+  bool dataTraverseStmtPre(Stmt *S) {
+// Do not show inlay hints for PseudoObjectExprs. They're never
+// genuine user codes in C++.
+//
+// For example, __builtin_dump_struct would expand to a PseudoObjectExpr
+// that includes a couple of calls to a printf function. Printing parameter
+// names for that anyway would end up with duplicate parameter names 
(which,
+// however, got de-duplicated after visiting) for the printf function.
+if (AST.getLangOpts().CPlusPlus && isa(S))
+  return false;
+return true;
+  }
+
   bool VisitCallExpr(CallExpr *E) {
 if (!Cfg.InlayHints.Parameters)
   return true;
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp 
b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 20c1cdd985dbc017..2b6c27b17b537a96 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1724,6 +1724,30 @@ TEST(InlayHints, RestrictRange) {
   ElementsAre(labelIs(": int"), labelIs(": char")));
 }
 
+TEST(ParameterHints, PseudoObjectExpr) {
+  Annotations Code(R"cpp(
+struct S {
+  __declspec(property(get=GetX, put=PutX)) int x[];
+  int GetX(int y, int z) { return 42 + y; }
+  void PutX(int y) { x = y; } // Not `x = y: y`
+};
+
+int printf(const char *Format, ...);
+
+int main() {
+  S s;
+  __builtin_dump_struct(&s, printf); // Not `Format: 
__builtin_dump_struct()`
+  printf($Param[["Hello, %d"]], 42); // Normal calls are not affected.
+  return s.x[1][2]; // Not `x[y: 1][z: 2]`
+}
+  )cpp");
+  auto TU = TestTU::withCode(Code.code());
+  TU.ExtraArgs.push_back("-fms-extensions");
+  auto AST = TU.build();
+  EXPECT_THAT(inlayHints(AST, std::nullopt),
+  ElementsAre(HintMatcher(ExpectedHint{"Format: ", "Param"}, 
Code)));
+}
+
 TEST(ParameterHints, ArgPacksAndConstructors) {
   assertParameterHints(
   R"cpp(

>From f15e35f67e23e742536ca2fe7229f5b6cba8f6b8 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Mon, 6 Nov 2023 17:32:50 +0800
Subject: [PATCH 2/4] Don't make the patch C++-specific

---
 clang-tools-extra/clangd/InlayHints.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 867c70e5dbc80c65..1b54b570c1c5d4fc 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -591,13 +591,13 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
 
   bool dataTraverseStmtPre(Stmt *S) {
 // Do not show inlay hints for PseudoObjectExprs. They're never
-// genuine user codes in C++.
+// genuine user codes.
 //
 // For example, __builtin_dump_struct would expand to a PseudoObjectExpr
 // that includes a couple of calls to a printf function. Printing parameter
 // names for that anyway would end up with duplicate parameter names 
(which,
 // however, got de-duplicated after visiting) for the printf function.
-if (AST.getLangOpts().CPlusPlus && isa(S))
+if (isa(S))
   return false;
 return true;
   }

>From 8813a4f11254f89feb72e435888d896911a5dc78 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Tue, 7 Nov 2023 14:22:07 +0800
Subject: [PATCH 3/4] Only apply the restriction to __builtin_dump_struct

---
 clang-tools-extra/clangd/InlayHints.cpp| 18 +-
 .../clangd/unittests/InlayHintTests.cpp|  9 ++---
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 1b54b570c1c5d4fc..9897d1c1e6da209e 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -590,15 +590,15 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
   }
 
   bool dataTraverseStmtPre(Stmt *S) {
-// Do not show inlay hints for PseudoObjectExprs. They're never
-// genuine user codes.
- 

[clang-tools-extra] [clangd] Don't show inlay hints for __builtin_dump_struct (PR #71366)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

Since the other PR turned out to be infeasible, I'm overriding 
`TraversePseudoObjectExpr` now.

https://github.com/llvm/llvm-project/pull/71366
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Don't show inlay hints for __builtin_dump_struct (PR #71366)

[Younan Zhang via cfe-commits]

@@ -1724,6 +1724,33 @@ TEST(InlayHints, RestrictRange) {
   ElementsAre(labelIs(": int"), labelIs(": char")));
 }
 
+TEST(ParameterHints, PseudoObjectExpr) {
+  Annotations Code(R"cpp(
+struct S {
+  __declspec(property(get=GetX, put=PutX)) int x[];
+  int GetX(int y, int z) { return 42 + y; }
+  void PutX(int y) { x = $one[[y]]; } // FIXME: Undesired `x = y: y` for 
this ill-formed expression.

zyn0217 wrote:

Sorry, I'm a bit inaccurate here: The assignment expression in `PutX` is not 
ill-formed but would expand to a recursive call to `PutX` since we're assigning 
to the member with the very setter. (Of course, this would end up crashing if 
we [run the program](https://gcc.godbolt.org/z/e3fW3W73a).)

I think I was suggesting that this is a bad case for supporting such an 
extension: the assignment expression could be of a syntactic form with an 
inappropriate hint location though.

https://github.com/llvm/llvm-project/pull/71366
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Don't show inlay hints for __builtin_dump_struct (PR #71366)

[Younan Zhang via cfe-commits]

@@ -1724,6 +1724,33 @@ TEST(InlayHints, RestrictRange) {
   ElementsAre(labelIs(": int"), labelIs(": char")));
 }
 
+TEST(ParameterHints, PseudoObjectExpr) {
+  Annotations Code(R"cpp(
+struct S {
+  __declspec(property(get=GetX, put=PutX)) int x[];
+  int GetX(int y, int z) { return 42 + y; }
+  void PutX(int y) { x = $one[[y]]; } // FIXME: Undesired `x = y: y` for 
this ill-formed expression.
+};
+
+int printf(const char *Format, ...);
+
+int main() {
+  S s;
+  __builtin_dump_struct(&s, printf); // Not `Format: 
__builtin_dump_struct()`
+  printf($Param[["Hello, %d"]], 42); // Normal calls are not affected.
+  return s.x[ $two[[1]] ][ $three[[2]] ]; // `x[y: 1][z: 2]`

zyn0217 wrote:

No problem.

https://github.com/llvm/llvm-project/pull/71366
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Don't show inlay hints for __builtin_dump_struct (PR #71366)

[Younan Zhang via cfe-commits]

@@ -589,6 +589,24 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
 return true;
   }
 
+  bool TraversePseudoObjectExpr(PseudoObjectExpr *E) {
+// Do not show inlay hints for the __builtin_dump_struct, which would
+// expand to a PseudoObjectExpr that includes a couple of calls to a
+// printf function. Printing parameter names for that anyway would end up
+// with duplicate parameter names (which, however, got de-duplicated after
+// visiting) for the printf function.
+if (auto *CE = dyn_cast(E->getSyntacticForm());
+CE && CE->getBuiltinCallee() == Builtin::BI__builtin_dump_struct)
+  // Only traverse the syntactic forms. This leaves the door open in case
+  // the arguments in the syntactic form for __builtin_dump_struct could
+  // possibly get parameter names.
+  return RecursiveASTVisitor::TraverseStmt(
+  E->getSyntacticForm());
+// FIXME: Shall we ignore semantic forms for other pseudo object

zyn0217 wrote:

I think this depends on our attitude towards parameter hints for other 
pseudo-expression usages. Take the getter case from the test for example, the 
expression for `S().x[1][2]` turns out to be
```
`-PseudoObjectExpr 'int'
 |- (Syntatic) MSPropertySubscriptExpr '' lvalue
 `- (Semantic) CXXMemberCallExpr 'int'
```
There's no chance to see hints for the arguments if we neglect all the 
syntactic forms being `CallExpr`s. Besides, there are still some similar usages 
in ObjC / OpenMP that I'm not familiar with. 
Anyway, if we agree to sacrifice the chances to see these hints, I'd love to 
move forward with all `CallExpr`s within a pseudo-expression being dropped. 
Otherwise, I think it's better to leave it to the next bug report / feature 
request. What do you think?

https://github.com/llvm/llvm-project/pull/71366
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Don't show inlay hints for __builtin_dump_struct (PR #71366)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/71366
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Don't show inlay hints for __builtin_dump_struct (PR #71366)

[Younan Zhang via cfe-commits]

@@ -1724,6 +1724,33 @@ TEST(InlayHints, RestrictRange) {
   ElementsAre(labelIs(": int"), labelIs(": char")));
 }
 
+TEST(ParameterHints, PseudoObjectExpr) {
+  Annotations Code(R"cpp(
+struct S {
+  __declspec(property(get=GetX, put=PutX)) int x[];
+  int GetX(int y, int z) { return 42 + y; }
+  void PutX(int y) { x = $one[[y]]; } // FIXME: Undesired `x = y: y` for 
this ill-formed expression.

zyn0217 wrote:

It's nice to wipe out these extra hints if it won't take too many steps. The 
AST shows,
```
`-PseudoObjectExpr 'void'
  |- (Syntactic) BinaryOperator
  `- (Semantic) CXXMemberCallExpr 'void'
```
I wonder if it is possible to enumerate all the bad cases we don't want (e.g. 
Don't walk down the expression if we encounter a `BinaryOperator` in 
`PseudoObjectExpr`.)

https://github.com/llvm/llvm-project/pull/71366
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Don't show inlay hints for __builtin_dump_struct (PR #71366)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 updated 
https://github.com/llvm/llvm-project/pull/71366

>From 808c141c34218dd542b00149216adc061567dd31 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Mon, 6 Nov 2023 16:50:02 +0800
Subject: [PATCH 1/5] [clangd] Don't show inlay hints for PseudoObjectExprs in
 C++

This closes https://github.com/clangd/clangd/issues/1813.

PseudoObjectExprs in C++ are currently not very interesting but probably
mess up inlay hints.
---
 clang-tools-extra/clangd/InlayHints.cpp   | 13 ++
 .../clangd/unittests/InlayHintTests.cpp   | 24 +++
 2 files changed, 37 insertions(+)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 3da047f95421385..867c70e5dbc80c6 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -589,6 +589,19 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
 return true;
   }
 
+  bool dataTraverseStmtPre(Stmt *S) {
+// Do not show inlay hints for PseudoObjectExprs. They're never
+// genuine user codes in C++.
+//
+// For example, __builtin_dump_struct would expand to a PseudoObjectExpr
+// that includes a couple of calls to a printf function. Printing parameter
+// names for that anyway would end up with duplicate parameter names 
(which,
+// however, got de-duplicated after visiting) for the printf function.
+if (AST.getLangOpts().CPlusPlus && isa(S))
+  return false;
+return true;
+  }
+
   bool VisitCallExpr(CallExpr *E) {
 if (!Cfg.InlayHints.Parameters)
   return true;
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp 
b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 20c1cdd985dbc01..2b6c27b17b537a9 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1724,6 +1724,30 @@ TEST(InlayHints, RestrictRange) {
   ElementsAre(labelIs(": int"), labelIs(": char")));
 }
 
+TEST(ParameterHints, PseudoObjectExpr) {
+  Annotations Code(R"cpp(
+struct S {
+  __declspec(property(get=GetX, put=PutX)) int x[];
+  int GetX(int y, int z) { return 42 + y; }
+  void PutX(int y) { x = y; } // Not `x = y: y`
+};
+
+int printf(const char *Format, ...);
+
+int main() {
+  S s;
+  __builtin_dump_struct(&s, printf); // Not `Format: 
__builtin_dump_struct()`
+  printf($Param[["Hello, %d"]], 42); // Normal calls are not affected.
+  return s.x[1][2]; // Not `x[y: 1][z: 2]`
+}
+  )cpp");
+  auto TU = TestTU::withCode(Code.code());
+  TU.ExtraArgs.push_back("-fms-extensions");
+  auto AST = TU.build();
+  EXPECT_THAT(inlayHints(AST, std::nullopt),
+  ElementsAre(HintMatcher(ExpectedHint{"Format: ", "Param"}, 
Code)));
+}
+
 TEST(ParameterHints, ArgPacksAndConstructors) {
   assertParameterHints(
   R"cpp(

>From f15e35f67e23e742536ca2fe7229f5b6cba8f6b8 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Mon, 6 Nov 2023 17:32:50 +0800
Subject: [PATCH 2/5] Don't make the patch C++-specific

---
 clang-tools-extra/clangd/InlayHints.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 867c70e5dbc80c6..1b54b570c1c5d4f 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -591,13 +591,13 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
 
   bool dataTraverseStmtPre(Stmt *S) {
 // Do not show inlay hints for PseudoObjectExprs. They're never
-// genuine user codes in C++.
+// genuine user codes.
 //
 // For example, __builtin_dump_struct would expand to a PseudoObjectExpr
 // that includes a couple of calls to a printf function. Printing parameter
 // names for that anyway would end up with duplicate parameter names 
(which,
 // however, got de-duplicated after visiting) for the printf function.
-if (AST.getLangOpts().CPlusPlus && isa(S))
+if (isa(S))
   return false;
 return true;
   }

>From 8813a4f11254f89feb72e435888d896911a5dc78 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Tue, 7 Nov 2023 14:22:07 +0800
Subject: [PATCH 3/5] Only apply the restriction to __builtin_dump_struct

---
 clang-tools-extra/clangd/InlayHints.cpp| 18 +-
 .../clangd/unittests/InlayHintTests.cpp|  9 ++---
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 1b54b570c1c5d4f..9897d1c1e6da209 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -590,15 +590,15 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
   }
 
   bool dataTraverseStmtPre(Stmt *S) {
-// Do not show inlay hints for PseudoObjectExprs. They're never
-// genuine user codes.
-//
- 

[clang-tools-extra] [clangd] Don't show inlay hints for __builtin_dump_struct (PR #71366)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 updated 
https://github.com/llvm/llvm-project/pull/71366

>From 808c141c34218dd542b00149216adc061567dd31 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Mon, 6 Nov 2023 16:50:02 +0800
Subject: [PATCH 1/6] [clangd] Don't show inlay hints for PseudoObjectExprs in
 C++

This closes https://github.com/clangd/clangd/issues/1813.

PseudoObjectExprs in C++ are currently not very interesting but probably
mess up inlay hints.
---
 clang-tools-extra/clangd/InlayHints.cpp   | 13 ++
 .../clangd/unittests/InlayHintTests.cpp   | 24 +++
 2 files changed, 37 insertions(+)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 3da047f95421385..867c70e5dbc80c6 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -589,6 +589,19 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
 return true;
   }
 
+  bool dataTraverseStmtPre(Stmt *S) {
+// Do not show inlay hints for PseudoObjectExprs. They're never
+// genuine user codes in C++.
+//
+// For example, __builtin_dump_struct would expand to a PseudoObjectExpr
+// that includes a couple of calls to a printf function. Printing parameter
+// names for that anyway would end up with duplicate parameter names 
(which,
+// however, got de-duplicated after visiting) for the printf function.
+if (AST.getLangOpts().CPlusPlus && isa(S))
+  return false;
+return true;
+  }
+
   bool VisitCallExpr(CallExpr *E) {
 if (!Cfg.InlayHints.Parameters)
   return true;
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp 
b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 20c1cdd985dbc01..2b6c27b17b537a9 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1724,6 +1724,30 @@ TEST(InlayHints, RestrictRange) {
   ElementsAre(labelIs(": int"), labelIs(": char")));
 }
 
+TEST(ParameterHints, PseudoObjectExpr) {
+  Annotations Code(R"cpp(
+struct S {
+  __declspec(property(get=GetX, put=PutX)) int x[];
+  int GetX(int y, int z) { return 42 + y; }
+  void PutX(int y) { x = y; } // Not `x = y: y`
+};
+
+int printf(const char *Format, ...);
+
+int main() {
+  S s;
+  __builtin_dump_struct(&s, printf); // Not `Format: 
__builtin_dump_struct()`
+  printf($Param[["Hello, %d"]], 42); // Normal calls are not affected.
+  return s.x[1][2]; // Not `x[y: 1][z: 2]`
+}
+  )cpp");
+  auto TU = TestTU::withCode(Code.code());
+  TU.ExtraArgs.push_back("-fms-extensions");
+  auto AST = TU.build();
+  EXPECT_THAT(inlayHints(AST, std::nullopt),
+  ElementsAre(HintMatcher(ExpectedHint{"Format: ", "Param"}, 
Code)));
+}
+
 TEST(ParameterHints, ArgPacksAndConstructors) {
   assertParameterHints(
   R"cpp(

>From f15e35f67e23e742536ca2fe7229f5b6cba8f6b8 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Mon, 6 Nov 2023 17:32:50 +0800
Subject: [PATCH 2/6] Don't make the patch C++-specific

---
 clang-tools-extra/clangd/InlayHints.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 867c70e5dbc80c6..1b54b570c1c5d4f 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -591,13 +591,13 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
 
   bool dataTraverseStmtPre(Stmt *S) {
 // Do not show inlay hints for PseudoObjectExprs. They're never
-// genuine user codes in C++.
+// genuine user codes.
 //
 // For example, __builtin_dump_struct would expand to a PseudoObjectExpr
 // that includes a couple of calls to a printf function. Printing parameter
 // names for that anyway would end up with duplicate parameter names 
(which,
 // however, got de-duplicated after visiting) for the printf function.
-if (AST.getLangOpts().CPlusPlus && isa(S))
+if (isa(S))
   return false;
 return true;
   }

>From 8813a4f11254f89feb72e435888d896911a5dc78 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Tue, 7 Nov 2023 14:22:07 +0800
Subject: [PATCH 3/6] Only apply the restriction to __builtin_dump_struct

---
 clang-tools-extra/clangd/InlayHints.cpp| 18 +-
 .../clangd/unittests/InlayHintTests.cpp|  9 ++---
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 1b54b570c1c5d4f..9897d1c1e6da209 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -590,15 +590,15 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
   }
 
   bool dataTraverseStmtPre(Stmt *S) {
-// Do not show inlay hints for PseudoObjectExprs. They're never
-// genuine user codes.
-//
- 

[clang-tools-extra] [clangd] Don't show inlay hints for __builtin_dump_struct (PR #71366)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/71366
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Defer the instantiation of explicit-specifier until constraint checking completes (PR #70548)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 commented:

Thank you for working on this! Some nit comments, hope you don't mind.
(Also invited some clang folks to have a detailed look at this. :=)


https://github.com/llvm/llvm-project/pull/70548
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Defer the instantiation of explicit-specifier until constraint checking completes (PR #70548)

[Younan Zhang via cfe-commits]

@@ -3553,6 +3553,56 @@ static unsigned getPackIndexForParam(Sema &S,
   llvm_unreachable("parameter index would not be produced from template");
 }
 
+// if `Specialization` is a `CXXConstructorDecl` or `CXXConversionDecl`
+// we try to instantiate and update its explicit specifier after constraint
+// checking.
+static Sema::TemplateDeductionResult
+tryInstantiateExplicitSpecifier(Sema &S, FunctionDecl *Specialization,

zyn0217 wrote:

nit: Do you think we should make this function a (private) member of `Sema` 
instead of a static function?

https://github.com/llvm/llvm-project/pull/70548
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Defer the instantiation of explicit-specifier until constraint checking completes (PR #70548)

[Younan Zhang via cfe-commits]

@@ -3553,6 +3553,56 @@ static unsigned getPackIndexForParam(Sema &S,
   llvm_unreachable("parameter index would not be produced from template");
 }
 
+// if `Specialization` is a `CXXConstructorDecl` or `CXXConversionDecl`
+// we try to instantiate and update its explicit specifier after constraint
+// checking.
+static Sema::TemplateDeductionResult
+tryInstantiateExplicitSpecifier(Sema &S, FunctionDecl *Specialization,
+const MultiLevelTemplateArgumentList 
&SubstArgs,
+TemplateDeductionInfo &Info,
+FunctionTemplateDecl *FunctionTemplate,
+ArrayRef DeducedArgs) {
+
+  const auto TryInstantiateExplicitSpecifierForSingleDecl =
+  [&](auto *ExplicitDecl) {
+ExplicitSpecifier ExplicitSpecifier =
+ExplicitDecl->getExplicitSpecifier();
+Expr *const Expr = ExplicitSpecifier.getExpr();
+if (!Expr) {
+  return Sema::TDK_Success;
+}
+if (!Expr->isValueDependent()) {
+  return Sema::TDK_Success;
+}
+// TemplateDeclInstantiator::InitFunctionInstantiation set the
+// ActiveInstType to TemplateInstantiation, but we need
+// to enable SFINAE when instantiating explicit specifier.

zyn0217 wrote:

```suggestion
// TemplateDeclInstantiator::InitFunctionInstantiation sets the
// ActiveInstType to TemplateInstantiation, but we need
// to enable SFINAE when instantiating an explicit specifier.
```

https://github.com/llvm/llvm-project/pull/70548
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Defer the instantiation of explicit-specifier until constraint checking completes (PR #70548)

[Younan Zhang via cfe-commits]

@@ -3553,6 +3553,56 @@ static unsigned getPackIndexForParam(Sema &S,
   llvm_unreachable("parameter index would not be produced from template");
 }
 
+// if `Specialization` is a `CXXConstructorDecl` or `CXXConversionDecl`
+// we try to instantiate and update its explicit specifier after constraint
+// checking.
+static Sema::TemplateDeductionResult
+tryInstantiateExplicitSpecifier(Sema &S, FunctionDecl *Specialization,
+const MultiLevelTemplateArgumentList 
&SubstArgs,
+TemplateDeductionInfo &Info,
+FunctionTemplateDecl *FunctionTemplate,
+ArrayRef DeducedArgs) {
+
+  const auto TryInstantiateExplicitSpecifierForSingleDecl =
+  [&](auto *ExplicitDecl) {
+ExplicitSpecifier ExplicitSpecifier =
+ExplicitDecl->getExplicitSpecifier();
+Expr *const Expr = ExplicitSpecifier.getExpr();
+if (!Expr) {
+  return Sema::TDK_Success;
+}
+if (!Expr->isValueDependent()) {
+  return Sema::TDK_Success;
+}
+// TemplateDeclInstantiator::InitFunctionInstantiation set the
+// ActiveInstType to TemplateInstantiation, but we need
+// to enable SFINAE when instantiating explicit specifier.
+Sema::InstantiatingTemplate Inst(
+S, Info.getLocation(), FunctionTemplate, DeducedArgs,
+Sema::CodeSynthesisContext::DeducedTemplateArgumentSubstitution,
+Info);
+const auto Instantiated =
+S.instantiateExplicitSpecifier(SubstArgs, ExplicitSpecifier);
+if (Instantiated.isInvalid()) {
+  ExplicitDecl->setInvalidDecl(true);
+  return clang::Sema::TDK_SubstitutionFailure;
+}
+ExplicitDecl->setExplicitSpecifier(Instantiated);
+return clang::Sema::TDK_Success;
+  };
+  Sema::TemplateDeductionResult DeductionResult = clang::Sema::TDK_Success;
+  if (CXXConstructorDecl *ConstructorDecl =
+  dyn_cast_or_null(Specialization)) {

zyn0217 wrote:

nit: use `dyn_cast` since we have ensured that `Specialization` is non-null at 
the caller site `FinishTemplateArgumentDeduction`.

https://github.com/llvm/llvm-project/pull/70548
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Defer the instantiation of explicit-specifier until constraint checking completes (PR #70548)

[Younan Zhang via cfe-commits]

@@ -3553,6 +3553,56 @@ static unsigned getPackIndexForParam(Sema &S,
   llvm_unreachable("parameter index would not be produced from template");
 }
 
+// if `Specialization` is a `CXXConstructorDecl` or `CXXConversionDecl`
+// we try to instantiate and update its explicit specifier after constraint
+// checking.
+static Sema::TemplateDeductionResult
+tryInstantiateExplicitSpecifier(Sema &S, FunctionDecl *Specialization,
+const MultiLevelTemplateArgumentList 
&SubstArgs,
+TemplateDeductionInfo &Info,
+FunctionTemplateDecl *FunctionTemplate,
+ArrayRef DeducedArgs) {
+
+  const auto TryInstantiateExplicitSpecifierForSingleDecl =
+  [&](auto *ExplicitDecl) {
+ExplicitSpecifier ExplicitSpecifier =
+ExplicitDecl->getExplicitSpecifier();
+Expr *const Expr = ExplicitSpecifier.getExpr();
+if (!Expr) {
+  return Sema::TDK_Success;
+}
+if (!Expr->isValueDependent()) {
+  return Sema::TDK_Success;
+}
+// TemplateDeclInstantiator::InitFunctionInstantiation set the
+// ActiveInstType to TemplateInstantiation, but we need
+// to enable SFINAE when instantiating explicit specifier.
+Sema::InstantiatingTemplate Inst(
+S, Info.getLocation(), FunctionTemplate, DeducedArgs,
+Sema::CodeSynthesisContext::DeducedTemplateArgumentSubstitution,
+Info);
+const auto Instantiated =
+S.instantiateExplicitSpecifier(SubstArgs, ExplicitSpecifier);
+if (Instantiated.isInvalid()) {
+  ExplicitDecl->setInvalidDecl(true);
+  return clang::Sema::TDK_SubstitutionFailure;
+}
+ExplicitDecl->setExplicitSpecifier(Instantiated);
+return clang::Sema::TDK_Success;
+  };
+  Sema::TemplateDeductionResult DeductionResult = clang::Sema::TDK_Success;
+  if (CXXConstructorDecl *ConstructorDecl =
+  dyn_cast_or_null(Specialization)) {
+DeductionResult =
+TryInstantiateExplicitSpecifierForSingleDecl(ConstructorDecl);
+  } else if (CXXConversionDecl *ConversionDecl =
+ dyn_cast_or_null(Specialization)) {
+DeductionResult =
+TryInstantiateExplicitSpecifierForSingleDecl(ConversionDecl);
+  }
+  return DeductionResult;

zyn0217 wrote:

Rather than returning `clang::Sema::TDK_Success` silently, shall we add an 
assertion that the `Specialization` is either a constructor or a conversion 
function?

https://github.com/llvm/llvm-project/pull/70548
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Defer the instantiation of explicit-specifier until constraint checking completes (PR #70548)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/70548
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Defer the instantiation of explicit-specifier until constraint checking completes (PR #70548)

[Younan Zhang via cfe-commits]

@@ -3553,6 +3553,49 @@ static unsigned getPackIndexForParam(Sema &S,
   llvm_unreachable("parameter index would not be produced from template");
 }
 
+// if `Specialization` is a `CXXConstructorDecl` or `CXXConversionDecl`
+// we try to instantiate and update its explicit specifier after constraint
+// checking.
+static Sema::TemplateDeductionResult
+resolveExplicitSpecifier(Sema &S, FunctionDecl *Specialization,
+ const MultiLevelTemplateArgumentList &SubstArgs,
+ TemplateDeductionInfo &Info,
+ FunctionTemplateDecl *FunctionTemplate,
+ ArrayRef DeducedArgs) {
+  auto GetExplicitSpecifier = [](FunctionDecl *D) {
+return isa(D)
+   ? cast(D)->getExplicitSpecifier()
+   : cast(D)->getExplicitSpecifier();
+  };
+  auto SetExplicitSpecifier = [](FunctionDecl *D, ExplicitSpecifier ES) {
+isa(D)
+? cast(D)->setExplicitSpecifier(ES)
+: cast(D)->setExplicitSpecifier(ES);
+  };
+
+  ExplicitSpecifier ES = GetExplicitSpecifier(Specialization);
+  Expr *const Expr = ES.getExpr();
+  if (!Expr) {
+return Sema::TDK_Success;
+  }
+  if (!Expr->isValueDependent()) {
+return Sema::TDK_Success;
+  }
+  // TemplateDeclInstantiator::InitFunctionInstantiation set the
+  // ActiveInstType to TemplateInstantiation, but we need
+  // to enable SFINAE when instantiating an explicit specifier.
+  Sema::InstantiatingTemplate Inst(

zyn0217 wrote:

> the InstantiatingTemplate here is only used to enable SFINAE...

I'm wondering if 
[`SFINAETrap`](https://github.com/llvm/llvm-project/blob/e6971e5a41fe30264af9a13c8f387c06f93c6d9c/clang/include/clang/Sema/Sema.h#L10006-L10007)
 does the trick for you.

https://github.com/llvm/llvm-project/pull/70548
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Defer the instantiation of explicit-specifier until constraint checking completes (PR #70548)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/70548
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Resolve the dependent type from its single instantiation. Take 1 (PR #71279)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 created 
https://github.com/llvm/llvm-project/pull/71279

This is an enhancement to the HeuristicResolver, trying to extract the deduced 
type from the single instantiation for a template. This partially addresses the 
point #1 from
https://github.com/clangd/clangd/issues/1768.

This patch doesn't tackle CXXUnresolvedConstructExpr or similarities since I 
feel that is more arduous and would prefer to leave it for my future work.

>From d73a8e2ee683e6812c21cb1de7363b14565a96d1 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Sat, 4 Nov 2023 18:43:58 +0800
Subject: [PATCH] [clangd] Resolve the dependent type from its single
 instantiation. Take 1

This is an enhancement to the HeuristicResolver, trying to extract
the deduced type from the single instantiation for a template. This
partially addresses the point #1 from
https://github.com/clangd/clangd/issues/1768.

This patch doesn't tackle CXXUnresolvedConstructExpr or similarities
since I feel that is more arduous and would prefer to leave it for
my future work.
---
 .../clangd/HeuristicResolver.cpp  | 101 ++
 .../clangd/unittests/XRefsTests.cpp   |  48 +
 2 files changed, 149 insertions(+)

diff --git a/clang-tools-extra/clangd/HeuristicResolver.cpp 
b/clang-tools-extra/clangd/HeuristicResolver.cpp
index 3c147b6b582bf0b..d3dced9b325367a 100644
--- a/clang-tools-extra/clangd/HeuristicResolver.cpp
+++ b/clang-tools-extra/clangd/HeuristicResolver.cpp
@@ -7,10 +7,14 @@
 
//===--===//
 
 #include "HeuristicResolver.h"
+#include "AST.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
 
 namespace clang {
@@ -46,6 +50,98 @@ const Type *resolveDeclsToType(const std::vector &Decls,
   return nullptr;
 }
 
+// Visitor that helps to extract deduced type from instantiated entities.
+// This merely performs the source location comparison against each Decl
+// until it finds a Decl with the same location as the
+// dependent one. Its associated type will then be extracted.
+struct InstantiatedDeclVisitor : RecursiveASTVisitor {
+
+  InstantiatedDeclVisitor(NamedDecl *DependentDecl) : 
DependentDecl(DependentDecl) {}
+
+  bool shouldVisitTemplateInstantiations() const { return true; }
+
+  bool shouldVisitLambdaBody() const { return true; }
+
+  bool shouldVisitImplicitCode() const { return true; }
+
+  template 
+  bool onDeclVisited(D *MaybeInstantiated) {
+if (MaybeInstantiated->getDeclContext()->isDependentContext())
+  return true;
+auto *Dependent = dyn_cast(DependentDecl);
+if (!Dependent)
+  return true;
+auto LHS = MaybeInstantiated->getTypeSourceInfo(),
+ RHS = Dependent->getTypeSourceInfo();
+if (!LHS || !RHS)
+  return true;
+if (LHS->getTypeLoc().getSourceRange() !=
+RHS->getTypeLoc().getSourceRange())
+  return true;
+DeducedType = MaybeInstantiated->getType();
+return false;
+  }
+
+  bool VisitFieldDecl(FieldDecl *FD) {
+return onDeclVisited(FD);
+  }
+
+  bool VisitVarDecl(VarDecl *VD) {
+return onDeclVisited(VD);
+  }
+
+  NamedDecl *DependentDecl;
+  QualType DeducedType;
+};
+
+/// Attempt to resolve the dependent type from the surrounding context for 
which
+/// a single instantiation is available.
+const Type *
+resolveTypeFromInstantiatedTemplate(const CXXDependentScopeMemberExpr *Expr) {
+  if (Expr->isImplicitAccess())
+return nullptr;
+
+  auto *Base = Expr->getBase();
+  NamedDecl *ND = nullptr;
+  if (auto *CXXMember = dyn_cast(Base))
+ND = CXXMember->getMemberDecl();
+
+  if (auto *DRExpr = dyn_cast(Base))
+ND = DRExpr->getFoundDecl();
+
+  // FIXME: Handle CXXUnresolvedConstructExpr. This kind of type doesn't have
+  // available Decls to be matched against. Which inhibits the current 
heuristic
+  // from resolving expressions such as `T().fo^o()`, where T is a
+  // single-instantiated template parameter.
+  if (!ND)
+return nullptr;
+
+  NamedDecl *Instantiation = nullptr;
+
+  // Find out a single instantiation that we can start with. The enclosing
+  // context for the current Decl might not be a templated entity (e.g. a 
member
+  // function inside a class template), hence we shall walk up the decl
+  // contexts first.
+  for (auto *EnclosingContext = ND->getDeclContext(); EnclosingContext;
+   EnclosingContext = EnclosingContext->getParent()) {
+if (auto *ND = dyn_cast(EnclosingContext)) {
+  Instantiation = getOnlyInstantiation(ND);
+  if (Instantiation)
+break;
+}
+  }
+
+  if (!Instantiation)
+return nullptr;
+
+  // This will traverse down the instantiation entity, visit each Decl, and
+  // extract the deduced type for the undetermined Decl `ND`.

[clang-tools-extra] [clangd] Resolve the dependent type from its single instantiation. Take 1 (PR #71279)

[Younan Zhang via cfe-commits]

@@ -46,6 +50,98 @@ const Type *resolveDeclsToType(const std::vector &Decls,
   return nullptr;
 }
 
+// Visitor that helps to extract deduced type from instantiated entities.
+// This merely performs the source location comparison against each Decl
+// until it finds a Decl with the same location as the
+// dependent one. Its associated type will then be extracted.
+struct InstantiatedDeclVisitor : RecursiveASTVisitor {
+
+  InstantiatedDeclVisitor(NamedDecl *DependentDecl) : 
DependentDecl(DependentDecl) {}
+
+  bool shouldVisitTemplateInstantiations() const { return true; }
+
+  bool shouldVisitLambdaBody() const { return true; }
+
+  bool shouldVisitImplicitCode() const { return true; }
+
+  template 
+  bool onDeclVisited(D *MaybeInstantiated) {
+if (MaybeInstantiated->getDeclContext()->isDependentContext())
+  return true;
+auto *Dependent = dyn_cast(DependentDecl);
+if (!Dependent)
+  return true;
+auto LHS = MaybeInstantiated->getTypeSourceInfo(),
+ RHS = Dependent->getTypeSourceInfo();
+if (!LHS || !RHS)
+  return true;
+if (LHS->getTypeLoc().getSourceRange() !=
+RHS->getTypeLoc().getSourceRange())
+  return true;
+DeducedType = MaybeInstantiated->getType();
+return false;
+  }
+
+  bool VisitFieldDecl(FieldDecl *FD) {
+return onDeclVisited(FD);
+  }
+
+  bool VisitVarDecl(VarDecl *VD) {
+return onDeclVisited(VD);
+  }
+
+  NamedDecl *DependentDecl;
+  QualType DeducedType;
+};
+
+/// Attempt to resolve the dependent type from the surrounding context for 
which
+/// a single instantiation is available.
+const Type *
+resolveTypeFromInstantiatedTemplate(const CXXDependentScopeMemberExpr *Expr) {
+  if (Expr->isImplicitAccess())
+return nullptr;
+
+  auto *Base = Expr->getBase();
+  NamedDecl *ND = nullptr;
+  if (auto *CXXMember = dyn_cast(Base))
+ND = CXXMember->getMemberDecl();
+
+  if (auto *DRExpr = dyn_cast(Base))
+ND = DRExpr->getFoundDecl();
+
+  // FIXME: Handle CXXUnresolvedConstructExpr. This kind of type doesn't have
+  // available Decls to be matched against. Which inhibits the current 
heuristic
+  // from resolving expressions such as `T().fo^o()`, where T is a
+  // single-instantiated template parameter.
+  if (!ND)
+return nullptr;

zyn0217 wrote:

RFC: Do we have a neat approach to deal with such an expression? One approach 
that comes to mind is: 

1) Extract the TemplateTypeParmDecl (TTPD)  from the expression. 

2) Traverse every Decls inside the enclosing DeclContext for TTPD until we find 
a template entity whose template parameters contain TTPD. 

3) Find the instantiation for that entity. If succeeds, perform the similar 
steps below to extract the Stmt corresponding to the dependent expression (i.e. 
our interest). 

4) Figure out a way to extract the type from that expression, which can be 
intricate, as we don't know to which Decl the dependent expression is tied.

https://github.com/llvm/llvm-project/pull/71279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Resolve the dependent type from its single instantiation. Take 1 (PR #71279)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/71279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Don't show inlay hints for PseudoObjectExprs in C++ (PR #71366)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 created 
https://github.com/llvm/llvm-project/pull/71366

This closes https://github.com/clangd/clangd/issues/1813.

PseudoObjectExprs in C++ are currently not very interesting but probably mess 
up inlay hints.

>From 4a878b63cbdd33833b998896120a992178438180 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Mon, 6 Nov 2023 16:50:02 +0800
Subject: [PATCH] [clangd] Don't show inlay hints for PseudoObjectExprs in C++

This closes https://github.com/clangd/clangd/issues/1813.

PseudoObjectExprs in C++ are currently not very interesting but probably
mess up inlay hints.
---
 clang-tools-extra/clangd/InlayHints.cpp   | 13 ++
 .../clangd/unittests/InlayHintTests.cpp   | 24 +++
 2 files changed, 37 insertions(+)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 3da047f95421385..867c70e5dbc80c6 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -589,6 +589,19 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
 return true;
   }
 
+  bool dataTraverseStmtPre(Stmt *S) {
+// Do not show inlay hints for PseudoObjectExprs. They're never
+// genuine user codes in C++.
+//
+// For example, __builtin_dump_struct would expand to a PseudoObjectExpr
+// that includes a couple of calls to a printf function. Printing parameter
+// names for that anyway would end up with duplicate parameter names 
(which,
+// however, got de-duplicated after visiting) for the printf function.
+if (AST.getLangOpts().CPlusPlus && isa(S))
+  return false;
+return true;
+  }
+
   bool VisitCallExpr(CallExpr *E) {
 if (!Cfg.InlayHints.Parameters)
   return true;
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp 
b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 20c1cdd985dbc01..2b6c27b17b537a9 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1724,6 +1724,30 @@ TEST(InlayHints, RestrictRange) {
   ElementsAre(labelIs(": int"), labelIs(": char")));
 }
 
+TEST(ParameterHints, PseudoObjectExpr) {
+  Annotations Code(R"cpp(
+struct S {
+  __declspec(property(get=GetX, put=PutX)) int x[];
+  int GetX(int y, int z) { return 42 + y; }
+  void PutX(int y) { x = y; } // Not `x = y: y`
+};
+
+int printf(const char *Format, ...);
+
+int main() {
+  S s;
+  __builtin_dump_struct(&s, printf); // Not `Format: 
__builtin_dump_struct()`
+  printf($Param[["Hello, %d"]], 42); // Normal calls are not affected.
+  return s.x[1][2]; // Not `x[y: 1][z: 2]`
+}
+  )cpp");
+  auto TU = TestTU::withCode(Code.code());
+  TU.ExtraArgs.push_back("-fms-extensions");
+  auto AST = TU.build();
+  EXPECT_THAT(inlayHints(AST, std::nullopt),
+  ElementsAre(HintMatcher(ExpectedHint{"Format: ", "Param"}, 
Code)));
+}
+
 TEST(ParameterHints, ArgPacksAndConstructors) {
   assertParameterHints(
   R"cpp(

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

[clang-tools-extra] [clangd] Don't show inlay hints for PseudoObjectExprs in C++ (PR #71366)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 updated 
https://github.com/llvm/llvm-project/pull/71366

>From 4a878b63cbdd33833b998896120a992178438180 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Mon, 6 Nov 2023 16:50:02 +0800
Subject: [PATCH 1/2] [clangd] Don't show inlay hints for PseudoObjectExprs in
 C++

This closes https://github.com/clangd/clangd/issues/1813.

PseudoObjectExprs in C++ are currently not very interesting but probably
mess up inlay hints.
---
 clang-tools-extra/clangd/InlayHints.cpp   | 13 ++
 .../clangd/unittests/InlayHintTests.cpp   | 24 +++
 2 files changed, 37 insertions(+)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 3da047f95421385..867c70e5dbc80c6 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -589,6 +589,19 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
 return true;
   }
 
+  bool dataTraverseStmtPre(Stmt *S) {
+// Do not show inlay hints for PseudoObjectExprs. They're never
+// genuine user codes in C++.
+//
+// For example, __builtin_dump_struct would expand to a PseudoObjectExpr
+// that includes a couple of calls to a printf function. Printing parameter
+// names for that anyway would end up with duplicate parameter names 
(which,
+// however, got de-duplicated after visiting) for the printf function.
+if (AST.getLangOpts().CPlusPlus && isa(S))
+  return false;
+return true;
+  }
+
   bool VisitCallExpr(CallExpr *E) {
 if (!Cfg.InlayHints.Parameters)
   return true;
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp 
b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 20c1cdd985dbc01..2b6c27b17b537a9 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1724,6 +1724,30 @@ TEST(InlayHints, RestrictRange) {
   ElementsAre(labelIs(": int"), labelIs(": char")));
 }
 
+TEST(ParameterHints, PseudoObjectExpr) {
+  Annotations Code(R"cpp(
+struct S {
+  __declspec(property(get=GetX, put=PutX)) int x[];
+  int GetX(int y, int z) { return 42 + y; }
+  void PutX(int y) { x = y; } // Not `x = y: y`
+};
+
+int printf(const char *Format, ...);
+
+int main() {
+  S s;
+  __builtin_dump_struct(&s, printf); // Not `Format: 
__builtin_dump_struct()`
+  printf($Param[["Hello, %d"]], 42); // Normal calls are not affected.
+  return s.x[1][2]; // Not `x[y: 1][z: 2]`
+}
+  )cpp");
+  auto TU = TestTU::withCode(Code.code());
+  TU.ExtraArgs.push_back("-fms-extensions");
+  auto AST = TU.build();
+  EXPECT_THAT(inlayHints(AST, std::nullopt),
+  ElementsAre(HintMatcher(ExpectedHint{"Format: ", "Param"}, 
Code)));
+}
+
 TEST(ParameterHints, ArgPacksAndConstructors) {
   assertParameterHints(
   R"cpp(

>From 240b7f89e9bc9eb57f35f43be2f971b4fde76b46 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Mon, 6 Nov 2023 17:32:50 +0800
Subject: [PATCH 2/2] Don't make the patch C++-specific

---
 clang-tools-extra/clangd/InlayHints.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index 867c70e5dbc80c6..1b54b570c1c5d4f 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -591,13 +591,13 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
 
   bool dataTraverseStmtPre(Stmt *S) {
 // Do not show inlay hints for PseudoObjectExprs. They're never
-// genuine user codes in C++.
+// genuine user codes.
 //
 // For example, __builtin_dump_struct would expand to a PseudoObjectExpr
 // that includes a couple of calls to a printf function. Printing parameter
 // names for that anyway would end up with duplicate parameter names 
(which,
 // however, got de-duplicated after visiting) for the printf function.
-if (AST.getLangOpts().CPlusPlus && isa(S))
+if (isa(S))
   return false;
 return true;
   }

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

[clang-tools-extra] [clangd] Don't show inlay hints for PseudoObjectExprs (PR #71366)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/71366
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Don't show inlay hints for PseudoObjectExprs (PR #71366)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/71366
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Concepts] Avoid substituting into constraints for invalid TemplateDecls (PR #75697)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 created 
https://github.com/llvm/llvm-project/pull/75697

Fixes https://github.com/llvm/llvm-project/issues/73885.

Substituting into constraints for invalid TemplateDecls might still yield 
dependent expressions and end up crashing later while attempting evaluation.

>From a6e01586f5d406b51492d973cb693ba32cc63d99 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Sat, 16 Dec 2023 18:49:59 +0800
Subject: [PATCH] [Concepts] Avoid substituting into constraints for invalid
 TemplateDecls

Fixes https://github.com/llvm/llvm-project/issues/73885.

Substituting into constraints for invalid TemplateDecls might still
yield dependent expressions and end up crashing later while
attempting evaluation.
---
 clang/docs/ReleaseNotes.rst   |  3 +++
 clang/lib/Sema/SemaConcept.cpp|  4 
 clang/test/SemaTemplate/instantiate-requires-expr.cpp | 10 ++
 3 files changed, 17 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b4b5352a306c1c..83f98b97925250 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -831,6 +831,9 @@ Bug Fixes to C++ Support
 - Fix crash when parsing nested requirement. Fixes:
   (`#73112 `_)
 
+- Fixed a crash when substituting into constraint expressions for invalid 
variable templates.
+  Fixes: (`#73885 `_)
+
 Bug Fixes to AST Handling
 ^
 - Fixed an import failure of recursive friend class template.
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 719c6aab74e017..73683fee7c1487 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -353,6 +353,10 @@ static ExprResult calculateConstraintSatisfaction(
   if (Inst.isInvalid())
 return ExprError();
 
+  // An empty expression for substitution failure messages.
+  if (Template && Template->isInvalidDecl())
+return ExprEmpty();
+
   llvm::FoldingSetNodeID ID;
   if (Template &&
   DiagRecursiveConstraintEval(S, ID, Template, AtomicExpr, MLTAL)) 
{
diff --git a/clang/test/SemaTemplate/instantiate-requires-expr.cpp 
b/clang/test/SemaTemplate/instantiate-requires-expr.cpp
index ba82fc1313fc95..fb4127182d1cb6 100644
--- a/clang/test/SemaTemplate/instantiate-requires-expr.cpp
+++ b/clang/test/SemaTemplate/instantiate-requires-expr.cpp
@@ -227,3 +227,13 @@ struct r6 {};
 
 using r6i = r6;
 // expected-error@-1 {{constraints not satisfied for class template 'r6' [with 
T = int]}}
+
+namespace GH73885 {
+template  // expected-error {{extraneous template parameter list}}
+template  // expected-error {{'T' shadows template parameter}}
+  // expected-note@-2 {{template parameter is declared 
here}}
+requires(T{})
+T e_v;
+double e = e_v;
+// expected-error@-1 {{constraints not satisfied for variable template 'e_v' 
[with T = double]}}
+}

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

[clang] [Concepts] Avoid substituting into constraints for invalid TemplateDecls (PR #75697)

[Younan Zhang via cfe-commits]

@@ -353,6 +353,10 @@ static ExprResult calculateConstraintSatisfaction(
   if (Inst.isInvalid())
 return ExprError();
 
+  // An empty expression for substitution failure messages.
+  if (Template && Template->isInvalidDecl())
+return ExprEmpty();

zyn0217 wrote:

My initial thought was to emit substitution details (`[T = double]` here). But 
now I feel like this is puzzling since we're now giving "constraints not 
satisfied" rather than "constraint must be of type 'bool'" in the case.

I'm a bit torn here: either we lose substitution details or produce inaccurate 
error-recovery messages. The latter matches what we're doing now in 
assertion-free builds. https://cpp1.godbolt.org/z/3KzPEYhn3

https://github.com/llvm/llvm-project/pull/75697
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang] [Sema][clangd] add noexcept to override functions during code completion (PR #75937)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

In addition to Nathan’s advice, I have a question about the commit message
> if the functions override a noexcept virtual function.
I didn't see anything reflecting this condition; are you still working on this 
patch? Would you mind adding a WIP prefix to the title before everything is 
ready? Thank you!

https://github.com/llvm/llvm-project/pull/75937
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [Sema][clangd] add noexcept to override functions during code completion (PR #75937)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

> ...which is called during code completion to generate the override function 
> declarator based on the FunctionDecl of the virtual function in the base.

The place you're patching is not only specific to "completing override 
functions", but handles all completion strings involving function declarations.

https://github.com/llvm/llvm-project/pull/75937
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [Sema][clangd] add noexcept to override functions during code completion (PR #75937)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

Bonus: It appears that neither gcc nor clang implements a provision change from 
[CWG1351](https://cplusplus.github.io/CWG/issues/1351.html),

> [except.spec]p4
> ..., **unless the overriding function is defined as deleted.**

giving errors on the following code.

```cpp
struct B {
  virtual void h() noexcept = delete;
};

struct D: B {
  void h() = delete;// Should be OK
};

int main() {
  D();
}
```
https://cpp2.godbolt.org/z/zvY17G6jr

https://github.com/llvm/llvm-project/pull/75937
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [Sema][clangd] add noexcept to override functions during code completion (PR #75937)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

> Maybe this deserves a new issue for clang Sema?

Sounds reasonable to me. Feel free to put up a PR / issue for this if you are 
interested.

https://github.com/llvm/llvm-project/pull/75937
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Use the materialized temporary's type while creating the APValue (PR #73355)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 created 
https://github.com/llvm/llvm-project/pull/73355

See https://github.com/llvm/llvm-project/issues/72025 for the bug and its 
diagnosis.

>From 3ff1b189cf55d3705b2823dc39eaaf710fa26541 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Sat, 25 Nov 2023 02:01:22 +0800
Subject: [PATCH] [clang] Use the materialized temporary's type while creating
 the APValue

See https://github.com/llvm/llvm-project/issues/72025 for the bug and
its diagnosis.
---
 clang/docs/ReleaseNotes.rst| 3 +++
 clang/lib/AST/ExprConstant.cpp | 2 +-
 clang/test/SemaCXX/pr72025.cpp | 9 +
 3 files changed, 13 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/SemaCXX/pr72025.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 74358219ba9fb22..d434d016907f815 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -691,6 +691,9 @@ Bug Fixes to C++ Support
   Fixes:
   (`#68769 `_)
 
+- Fixed a crash for C++98/03 while checking an ill-formed ``_Static_assert`` 
expression.
+  Fixes: (`#72025 `_)
+
 - Clang now defers the instantiation of explicit specifier until constraint 
checking
   completes (except deduction guides). Fixes:
   (`#59827 `_)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index e16fec6109e744e..6c6ad12119d13c3 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -8607,7 +8607,7 @@ bool LValueExprEvaluator::VisitMaterializeTemporaryExpr(
 Result.set(E);
   } else {
 Value = &Info.CurrentCall->createTemporary(
-E, E->getType(),
+E, Inner->getType(),
 E->getStorageDuration() == SD_FullExpression ? 
ScopeKind::FullExpression
  : ScopeKind::Block,
 Result);
diff --git a/clang/test/SemaCXX/pr72025.cpp b/clang/test/SemaCXX/pr72025.cpp
new file mode 100644
index 000..9f0a4b0f43630c5
--- /dev/null
+++ b/clang/test/SemaCXX/pr72025.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -verify -std=c++03 -fsyntax-only %s
+struct V {
+  char c[2];
+  banana V() : c("i") {} // expected-error {{unknown type name}}
+ // expected-error@-1 {{constructor cannot have a 
return type}}
+};
+
+_Static_assert(V().c[0], ""); // expected-error {{is not an integral constant 
expression}}
+

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

[clang-tools-extra] [clangd] Resolve the dependent type from its single instantiation. Take 1 (PR #71279)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

Thank you again for bothering with this, and sorry for not responding for over 
a week.

Just now, I replicated the experimentation

> I've done some local experimentation, and what I'm seeing is that 
> `TemplateTypeParmDecl::getDeclContext()` does return the FunctionDecl or 
> CXXRecordDecl which owns the template parameter.

 and had to admit that I was wrong previously. Something must be wrong with my 
recollection, but I could clearly remember that I'd run into the same pitfall 
my first time. (I was about to write the comment to highlight this, but soon, I 
gave up taking this approach since it looks more constricted than I thought. 
And here I'm explaining it.)

> One tricky thing I found is that when starting from a template parameter 
> type, it's important to use dyn_cast(T) rather than 
> T->getAs(); the latter does more than just cast, it 
> returns the canonical type whose associated Decl is null.

The contrived example involving template template parameter is one obstruction; 
Another more common example could be such. (Which is simplified from the test 
case):

```cpp
template 
struct Outer {
  template 
  void foo(U arg) {
 arg.fo^o();
  }
};

struct Widget {
  void foo();
};

Outer().foo(Widget());
```

Here, we want to find the only instantiation before locating `foo().` Which is 
`Outer::foo.` To locate that, we shall find the templated Decl for 
`foo`, which could be done by conducting the `ParmVarDecl -> 
TemplateParmVarDecl -> getDeclContext -> FunctionTemplateDecl` dance. 
Everything goes well until the specialization set for the 
`FunctionTemplateDecl` is **empty**. Hence, our `getOnlyInstantiation` returns 
null, which is crazy!

This is because when clang performs the instantiation for member templates, it 
creates a new `FunctionTemplateDecl` from the primary template (i.e. where we 
start from) and links the instantiation declaration to that new `Decl`.

https://github.com/llvm/llvm-project/blob/1449b349ac4072adb1f593234c1d6f67986d3b6a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp#L2184-L2203

And what makes it worse is, as of now, we don't have any approach to retrieve 
the new templated `Decl` from the primary `Decl`:

https://github.com/llvm/llvm-project/blob/1449b349ac4072adb1f593234c1d6f67986d3b6a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp#L4789-L4803

("// FIXME: New needs a pointer to Tmpl", I was quite frustrated when I read 
it!)

(This patch first walks up the `DeclContext` to collect the very outer `Decl` 
for templates; then, the visitor could access the later-created template decl 
and its instantiation from the enclosing template. Again, I didn't realize the 
visitor resolved this issue this way (by accident), so I'm sorry for not being 
so straightforward.)

> The implementation of "Approach 2" could look similar to the current 
> InstantiatedDeclVisitor in the patch, except the node type we are searching 
> for is not limited to a Decl, it can also be other node types like Stmt etc.

Exactly. And this is why the patch is marked as "Take 1" :)

> However, the entry point to the check can be the same (e.g. for resolving a 
> dependent member expr, we'd want to do the "only instantiation" check in the 
> same cases where we call HeuristicResolver::resolveMemberExpr() from external 
> code), so the functionality could still live in HeuristicResolver for 
> organizational purposes.

Yeah, I'd love to extend the HeuristicResolver to make it more accurate and not 
just dependent on the mere name lookup.

https://github.com/llvm/llvm-project/pull/71279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Resolve the dependent type from its single instantiation. Take 1 (PR #71279)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

> I also discovered some complications related to nested templates, and I have 
> some thoughts on how to resolve them, but I'm going to suggest that we start 
> with getting a simple case (e.g. just one level of templates, no nested 
> templates) to work, and then tackle nested templates as a next step.

Oops, I replied too quickly and I should have noticed this paragraph. Does my 
previous comment elaborate on the same issue you've encountered? Could you 
kindly share your thoughts more?

https://github.com/llvm/llvm-project/pull/71279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Use the materialized temporary's type while creating the APValue (PR #73355)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

Ping. And invited @cor3ntin to kindly take a look at this.

https://github.com/llvm/llvm-project/pull/73355
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Use the materialized temporary's type while creating the APValue (PR #73355)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

The code-formatter failed on other unrelated lines, so I think I'd better not 
to touch them.

https://github.com/llvm/llvm-project/pull/73355
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Don't show inlay hints for __builtin_dump_struct (PR #71366)

[Younan Zhang via cfe-commits]

@@ -589,6 +589,24 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
 return true;
   }
 
+  bool TraversePseudoObjectExpr(PseudoObjectExpr *E) {
+// Do not show inlay hints for the __builtin_dump_struct, which would
+// expand to a PseudoObjectExpr that includes a couple of calls to a
+// printf function. Printing parameter names for that anyway would end up
+// with duplicate parameter names (which, however, got de-duplicated after
+// visiting) for the printf function.
+if (auto *CE = dyn_cast(E->getSyntacticForm());
+CE && CE->getBuiltinCallee() == Builtin::BI__builtin_dump_struct)
+  // Only traverse the syntactic forms. This leaves the door open in case
+  // the arguments in the syntactic form for __builtin_dump_struct could
+  // possibly get parameter names.
+  return RecursiveASTVisitor::TraverseStmt(
+  E->getSyntacticForm());
+// FIXME: Shall we ignore semantic forms for other pseudo object

zyn0217 wrote:

Oops. sorry for messing up the types. :-(

https://github.com/llvm/llvm-project/pull/71366
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Use the materialized temporary's type while creating the APValue (PR #73355)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 closed 
https://github.com/llvm/llvm-project/pull/73355
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Resolve the dependent type from its single instantiation. Take 1 (PR #71279)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

Another issue I've encountered while at it is that, given the following code,
```cpp
void foo();
auto lambda = [] {
  return ^foo();
};
```
let `N` represent the selection node for the expression `foo()`, 
`N.getDeclContext()` then yields `TranslationUnitDecl` rather than the 
`CXXRecordDecl`of the lambda expression.

I presume there's something wrong at `SelectionTree::Node::getDeclContext()`, 
because lambda expressions would slip through the current traversal logic.

```txt
`-LambdaExpr  '(lambda at line:2:15)'<-- parent 3; not a 
Decl thus ignored!
  |-CXXRecordDecl  col:15 implicit class definition
  | |-DefinitionData
  | | |-DefaultConstructor
  | | |-CopyConstructor
  | | |-MoveConstructor
  | | |-CopyAssignment
  | | |-MoveAssignment
  | | `-Destructor
  | |-CXXMethodDecl  line:2:15 constexpr operator() 'auto () 
const -> void' inline
  | | `-CompoundStmt 
  | |   `-ReturnStmt 
  | | `-CallExpr  'void'
  | |   `-ImplicitCastExpr  'void (*)()' 
  | | `-DeclRefExpr  'void ()' lvalue Function 0xb76c1c8 'foo' 
'void ()'
  | |-CXXConversionDecl  line:2:15 implicit constexpr 
operator void (*)() 'auto (*() const noexcept)() -> void' inline
  | |-CXXMethodDecl  line:2:15 implicit __invoke 'auto () -> 
void' static inline
  | `-CXXDestructorDecl  col:15 implicit referenced constexpr ~(lambda 
at line:2:15) 'void () noexcept' inline default trivial
  `-CompoundStmt<--parent 2
`-ReturnStmt<-- parent 1
  `-CallExpr  'void'   <-- starting point
`-ImplicitCastExpr  'void (*)()' 
  `-DeclRefExpr  'void ()' lvalue Function 0xb76c1c8 'foo' 
'void ()'
```

https://github.com/llvm/llvm-project/blob/51b988efb06f0343e7b71c9aec9ec3195412179d/clang-tools-extra/clangd/Selection.cpp#L1107-L1118

https://github.com/llvm/llvm-project/pull/71279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Handle lambda scopes inside Node::getDeclContext() (PR #76329)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 created 
https://github.com/llvm/llvm-project/pull/76329

We used to consider the `DeclContext` for selection nodes inside a lambda as 
the enclosing scope of the lambda expression, rather than the lambda itself.

For example,

```cpp
void foo();
auto lambda = [] {
  return ^foo();
};
```

where `N` is the selection node for the expression `foo()`, 
`N.getDeclContext()` returns the `TranslationUnitDecl` previously, which IMO is 
wrong, since the `RecordDecl` of the lambda is closer.

Seemingly, this affects nothing currently.

>From c0b04507075b39e494d3e6fa39168bc47b92f338 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Sun, 24 Dec 2023 18:08:30 +0800
Subject: [PATCH] [clangd] Handle lambda scopes inside Node::getDeclContext()

We used to consider the DeclContext for selection nodes inside
a lambda as the scope of the lambda expression locates in.

For example,

```cpp
void foo();
auto lambda = [] {
  return ^foo();
};
```

where N is the selection node for the expression `foo()`,
`N.getDeclContext()` returns the TranslationUnitDecl previously,
which IMO is wrong, since the RecordDecl of the lambda is closer.
---
 clang-tools-extra/clangd/Selection.cpp  |  3 +++
 .../clangd/unittests/SelectionTests.cpp | 13 +
 2 files changed, 16 insertions(+)

diff --git a/clang-tools-extra/clangd/Selection.cpp 
b/clang-tools-extra/clangd/Selection.cpp
index 8c6d5750ecefdb..a7413485b04ac3 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -1113,6 +1113,9 @@ const DeclContext &SelectionTree::Node::getDeclContext() 
const {
   return *DC;
   return *Current->getLexicalDeclContext();
 }
+if (auto *LE = CurrentNode->ASTNode.get())
+  if (CurrentNode != this)
+return *LE->getLambdaClass();
   }
   llvm_unreachable("A tree must always be rooted at TranslationUnitDecl.");
 }
diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp 
b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index 4c019a1524f3c3..cdb50dfc1c3095 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -880,6 +880,19 @@ TEST(SelectionTest, DeclContextIsLexical) {
   }
 }
 
+TEST(SelectionTest, DeclContextLambda) {
+  llvm::Annotations Test(R"cpp(
+void foo();
+auto lambda = [] {
+  return $1^foo();
+};
+  )cpp");
+  auto AST = TestTU::withCode(Test.code()).build();
+  auto ST = SelectionTree::createRight(AST.getASTContext(), AST.getTokens(),
+   Test.point("1"), Test.point("1"));
+  EXPECT_FALSE(ST.commonAncestor()->getDeclContext().isTranslationUnit());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

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

[clang-tools-extra] [WIP][clangd] Resolve the dependent type from its single instantiation. Take 1 (PR #71279)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/71279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [WIP][clangd] Resolve the dependent type from its single instantiation. Take 1 (PR #71279)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 converted_to_draft 
https://github.com/llvm/llvm-project/pull/71279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [WIP][clangd] Resolve the dependent type from its single instantiation. Take 1 (PR #71279)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 updated 
https://github.com/llvm/llvm-project/pull/71279

>From c0703d7d9549e82434b37f9d5658b566a480d752 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Sat, 4 Nov 2023 18:43:58 +0800
Subject: [PATCH 1/4] [clangd] Resolve the dependent type from its single
 instantiation. Take 1

This is an enhancement to the HeuristicResolver, trying to extract
the deduced type from the single instantiation for a template. This
partially addresses the point #1 from
https://github.com/clangd/clangd/issues/1768.

This patch doesn't tackle CXXUnresolvedConstructExpr or similarities
since I feel that is more arduous and would prefer to leave it for
my future work.
---
 .../clangd/HeuristicResolver.cpp  | 101 ++
 .../clangd/unittests/XRefsTests.cpp   |  48 +
 2 files changed, 149 insertions(+)

diff --git a/clang-tools-extra/clangd/HeuristicResolver.cpp 
b/clang-tools-extra/clangd/HeuristicResolver.cpp
index 3c147b6b582bf0..d3dced9b325367 100644
--- a/clang-tools-extra/clangd/HeuristicResolver.cpp
+++ b/clang-tools-extra/clangd/HeuristicResolver.cpp
@@ -7,10 +7,14 @@
 
//===--===//
 
 #include "HeuristicResolver.h"
+#include "AST.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
 
 namespace clang {
@@ -46,6 +50,98 @@ const Type *resolveDeclsToType(const std::vector &Decls,
   return nullptr;
 }
 
+// Visitor that helps to extract deduced type from instantiated entities.
+// This merely performs the source location comparison against each Decl
+// until it finds a Decl with the same location as the
+// dependent one. Its associated type will then be extracted.
+struct InstantiatedDeclVisitor : RecursiveASTVisitor {
+
+  InstantiatedDeclVisitor(NamedDecl *DependentDecl) : 
DependentDecl(DependentDecl) {}
+
+  bool shouldVisitTemplateInstantiations() const { return true; }
+
+  bool shouldVisitLambdaBody() const { return true; }
+
+  bool shouldVisitImplicitCode() const { return true; }
+
+  template 
+  bool onDeclVisited(D *MaybeInstantiated) {
+if (MaybeInstantiated->getDeclContext()->isDependentContext())
+  return true;
+auto *Dependent = dyn_cast(DependentDecl);
+if (!Dependent)
+  return true;
+auto LHS = MaybeInstantiated->getTypeSourceInfo(),
+ RHS = Dependent->getTypeSourceInfo();
+if (!LHS || !RHS)
+  return true;
+if (LHS->getTypeLoc().getSourceRange() !=
+RHS->getTypeLoc().getSourceRange())
+  return true;
+DeducedType = MaybeInstantiated->getType();
+return false;
+  }
+
+  bool VisitFieldDecl(FieldDecl *FD) {
+return onDeclVisited(FD);
+  }
+
+  bool VisitVarDecl(VarDecl *VD) {
+return onDeclVisited(VD);
+  }
+
+  NamedDecl *DependentDecl;
+  QualType DeducedType;
+};
+
+/// Attempt to resolve the dependent type from the surrounding context for 
which
+/// a single instantiation is available.
+const Type *
+resolveTypeFromInstantiatedTemplate(const CXXDependentScopeMemberExpr *Expr) {
+  if (Expr->isImplicitAccess())
+return nullptr;
+
+  auto *Base = Expr->getBase();
+  NamedDecl *ND = nullptr;
+  if (auto *CXXMember = dyn_cast(Base))
+ND = CXXMember->getMemberDecl();
+
+  if (auto *DRExpr = dyn_cast(Base))
+ND = DRExpr->getFoundDecl();
+
+  // FIXME: Handle CXXUnresolvedConstructExpr. This kind of type doesn't have
+  // available Decls to be matched against. Which inhibits the current 
heuristic
+  // from resolving expressions such as `T().fo^o()`, where T is a
+  // single-instantiated template parameter.
+  if (!ND)
+return nullptr;
+
+  NamedDecl *Instantiation = nullptr;
+
+  // Find out a single instantiation that we can start with. The enclosing
+  // context for the current Decl might not be a templated entity (e.g. a 
member
+  // function inside a class template), hence we shall walk up the decl
+  // contexts first.
+  for (auto *EnclosingContext = ND->getDeclContext(); EnclosingContext;
+   EnclosingContext = EnclosingContext->getParent()) {
+if (auto *ND = dyn_cast(EnclosingContext)) {
+  Instantiation = getOnlyInstantiation(ND);
+  if (Instantiation)
+break;
+}
+  }
+
+  if (!Instantiation)
+return nullptr;
+
+  // This will traverse down the instantiation entity, visit each Decl, and
+  // extract the deduced type for the undetermined Decl `ND`.
+  InstantiatedDeclVisitor Visitor(ND);
+  Visitor.TraverseDecl(Instantiation);
+
+  return Visitor.DeducedType.getTypePtrOrNull();
+}
+
 } // namespace
 
 // Helper function for HeuristicResolver::resolveDependentMember()
@@ -150,6 +246,11 @@ std::vector 
HeuristicResolver::resolveMemberExpr(
   if (ME->isArrow()) {
 BaseType = getPointeeType(BaseType);
   }
+
+  if 

[clang-tools-extra] [clangd] Handle lambda scopes inside Node::getDeclContext() (PR #76329)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 updated 
https://github.com/llvm/llvm-project/pull/76329

>From c0b04507075b39e494d3e6fa39168bc47b92f338 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Sun, 24 Dec 2023 18:08:30 +0800
Subject: [PATCH 1/2] [clangd] Handle lambda scopes inside
 Node::getDeclContext()

We used to consider the DeclContext for selection nodes inside
a lambda as the scope of the lambda expression locates in.

For example,

```cpp
void foo();
auto lambda = [] {
  return ^foo();
};
```

where N is the selection node for the expression `foo()`,
`N.getDeclContext()` returns the TranslationUnitDecl previously,
which IMO is wrong, since the RecordDecl of the lambda is closer.
---
 clang-tools-extra/clangd/Selection.cpp  |  3 +++
 .../clangd/unittests/SelectionTests.cpp | 13 +
 2 files changed, 16 insertions(+)

diff --git a/clang-tools-extra/clangd/Selection.cpp 
b/clang-tools-extra/clangd/Selection.cpp
index 8c6d5750ecefdb..a7413485b04ac3 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -1113,6 +1113,9 @@ const DeclContext &SelectionTree::Node::getDeclContext() 
const {
   return *DC;
   return *Current->getLexicalDeclContext();
 }
+if (auto *LE = CurrentNode->ASTNode.get())
+  if (CurrentNode != this)
+return *LE->getLambdaClass();
   }
   llvm_unreachable("A tree must always be rooted at TranslationUnitDecl.");
 }
diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp 
b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index 4c019a1524f3c3..cdb50dfc1c3095 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -880,6 +880,19 @@ TEST(SelectionTest, DeclContextIsLexical) {
   }
 }
 
+TEST(SelectionTest, DeclContextLambda) {
+  llvm::Annotations Test(R"cpp(
+void foo();
+auto lambda = [] {
+  return $1^foo();
+};
+  )cpp");
+  auto AST = TestTU::withCode(Test.code()).build();
+  auto ST = SelectionTree::createRight(AST.getASTContext(), AST.getTokens(),
+   Test.point("1"), Test.point("1"));
+  EXPECT_FALSE(ST.commonAncestor()->getDeclContext().isTranslationUnit());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

>From 18274ed552f955172a788391393c58f8e32a65a6 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Sun, 24 Dec 2023 23:50:10 +0800
Subject: [PATCH 2/2] DC should be operator()

---
 clang-tools-extra/clangd/Selection.cpp| 4 ++--
 clang-tools-extra/clangd/unittests/SelectionTests.cpp | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clangd/Selection.cpp 
b/clang-tools-extra/clangd/Selection.cpp
index a7413485b04ac3..277cb8769a1b12 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -1113,9 +1113,9 @@ const DeclContext &SelectionTree::Node::getDeclContext() 
const {
   return *DC;
   return *Current->getLexicalDeclContext();
 }
-if (auto *LE = CurrentNode->ASTNode.get())
+if (const auto *LE = CurrentNode->ASTNode.get())
   if (CurrentNode != this)
-return *LE->getLambdaClass();
+return *LE->getCallOperator();
   }
   llvm_unreachable("A tree must always be rooted at TranslationUnitDecl.");
 }
diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp 
b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index cdb50dfc1c3095..754e8c287c5148 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -890,7 +890,7 @@ TEST(SelectionTest, DeclContextLambda) {
   auto AST = TestTU::withCode(Test.code()).build();
   auto ST = SelectionTree::createRight(AST.getASTContext(), AST.getTokens(),
Test.point("1"), Test.point("1"));
-  EXPECT_FALSE(ST.commonAncestor()->getDeclContext().isTranslationUnit());
+  EXPECT_TRUE(ST.commonAncestor()->getDeclContext().isFunctionOrMethod());
 }
 
 } // namespace

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

[clang-tools-extra] [clangd] Handle lambda scopes inside Node::getDeclContext() (PR #76329)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/76329
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Correctly implement CWG 2672 (PR #75001)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

friendly ping~

https://github.com/llvm/llvm-project/pull/75001
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Handle lambda scopes inside Node::getDeclContext() (PR #76329)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

Ping. And Happy New Year 2024!

https://github.com/llvm/llvm-project/pull/76329
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] [clangd] Fix is spelled in source bug (PR #76668)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 commented:

Thanks for your contribution! A couple of nit-picks, and I’d leave the approval 
to other folks.

https://github.com/llvm/llvm-project/pull/76668
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [clang] [clang-tools-extra] [clangd] Fix is spelled in source bug (PR #76668)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/76668
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [clang-tools-extra] [clang] [clangd] Fix is spelled in source bug (PR #76668)

[Younan Zhang via cfe-commits]

@@ -813,6 +813,21 @@ TEST(SourceCodeTests, isKeywords) {
   EXPECT_FALSE(isKeyword("override", LangOpts));
 }
 
+TEST(SourceCodeTests, isSpelledInSource) {
+  Annotations Test(R"cpp(
+int abc = 1;
+)cpp");
+
+  ParsedAST AST = TestTU::withCode(Test.code()).build();
+  llvm::errs() << Test.code();

zyn0217 wrote:

Debugging purpose?

https://github.com/llvm/llvm-project/pull/76668
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [clang] [clang-tools-extra] [clangd] Fix is spelled in source bug (PR #76668)

[Younan Zhang via cfe-commits]

@@ -232,7 +232,12 @@ bool isSpelledInSource(SourceLocation Loc, const 
SourceManager &SM) {
   if (Loc.isFileID())
 return true;
   auto Spelling = SM.getDecomposedSpellingLoc(Loc);
-  StringRef SpellingFile = SM.getSLocEntry(Spelling.first).getFile().getName();
+  bool InvalidSLocEntry = false;
+  const auto SLocEntry = SM.getSLocEntry(Spelling.first, &InvalidSLocEntry);
+  if (InvalidSLocEntry) {
+return false;
+  }

zyn0217 wrote:

```suggestion
  if (InvalidSLocEntry)
return false;
```
We don't usually add braces to one-line-if statements.

See 
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements.

https://github.com/llvm/llvm-project/pull/76668
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [clang] [clang-tools-extra] [clangd] Fix is spelled in source bug (PR #76668)

[Younan Zhang via cfe-commits]

@@ -232,7 +232,12 @@ bool isSpelledInSource(SourceLocation Loc, const 
SourceManager &SM) {
   if (Loc.isFileID())
 return true;
   auto Spelling = SM.getDecomposedSpellingLoc(Loc);
-  StringRef SpellingFile = SM.getSLocEntry(Spelling.first).getFile().getName();
+  bool InvalidSLocEntry = false;
+  const auto SLocEntry = SM.getSLocEntry(Spelling.first, &InvalidSLocEntry);
+  if (InvalidSLocEntry) {
+return false;
+  }
+  const StringRef SpellingFile = SLocEntry.getFile().getName();

zyn0217 wrote:

The const qualifier looks unnecessary to me. Could you please leave it out?

https://github.com/llvm/llvm-project/pull/76668
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Correctly construct template arguments for file-scope template template parameters (PR #76811)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 created 
https://github.com/llvm/llvm-project/pull/76811

This fixes the bug introduced by
https://github.com/llvm/llvm-project/commit/6db007a0654ed7a6ed5c3aa3b61a937c19a6bc6b.

We construct placeholder template arguments for template-template parameters to 
avoid mismatching argument substitution since they have different depths with 
their corresponding template arguments. In this case,

```cpp
template  class T> void foo(T);
```

T lies at the depth 0, and C lies at 1. The corresponding argument, of which 
there is exactly one, int, is at depth 0. If we consider the argument as the 
outermost one, then we would end up substituting 'int' into the wrong parameter 
T.

We used to perform such placeholder construction during the context walk-up. In 
the previous patch, we slipped through that inadvertently because we would walk 
up to the parent, which is precisely a FileContext for template-template 
parameters, after adding innermost arguments.

Besides, this patch moves the sanity check up to the context switch. That way, 
we avoid dereferencing null pointers if ND is unspecified.

Closes https://github.com/llvm/llvm-project/issues/57410.

>From 1164c705a8515d39bc9d4404e8523da8876d81cf Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Wed, 3 Jan 2024 19:33:01 +0800
Subject: [PATCH] [Clang] Correctly construct template arguments for file-scope
 template template parameters

This fixes the bug introduced by
https://github.com/llvm/llvm-project/commit/6db007a0654ed7a6ed5c3aa3b61a937c19a6bc6b.

We construct placeholder template arguments for template-template parameters to
avoid mismatching argument substitution since they have different depths
with their corresponding template arguments. In this case,

```cpp
template  class T> void foo(T);
```

T lies at the depth 0, and C lies at 1. The corresponding argument, of which
there is exactly one, int, is at depth 0. If we consider the
argument as the outermost one, then we would end up substituting 'int'
into the wrong parameter T.

We used to perform such placeholder construction during the context walk-up.
In the previous patch, we slipped through that inadvertently because we would
walk up to the parent, which is precisely a FileContext for template-template
parameters, after adding innermost arguments.

Besides, this patch moves the sanity check up to the context switch.
That way, we avoid dereferencing null pointers if ND is unspecified.

Closes https://github.com/llvm/llvm-project/issues/57410.
---
 clang/docs/ReleaseNotes.rst   |  3 +++
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 12 ++---
 .../temp/temp.arg/temp.arg.template/p3-2a.cpp | 25 +++
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0c8fec691bf3c9..7193d711333780 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -843,6 +843,9 @@ Bug Fixes to C++ Support
 - Fix crash when parsing nested requirement. Fixes:
   (`#73112 `_)
 
+- Fix a regression where clang forgets how to substitute into constraints on 
template-template
+  parameters. Fixes: (`#57410 
`_)
+
 Bug Fixes to AST Handling
 ^
 - Fixed an import failure of recursive friend class template.
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp 
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index df6b40999e645c..4420280efebb86 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -345,15 +345,19 @@ MultiLevelTemplateArgumentList 
Sema::getTemplateInstantiationArgs(
 
   using namespace TemplateInstArgsHelpers;
   const Decl *CurDecl = ND;
+
+  if (!ND)
+CurDecl = Decl::castFromDeclContext(DC);
+
   if (Innermost) {
 Result.addOuterTemplateArguments(const_cast(ND),
  Innermost->asArray(), Final);
-CurDecl = Response::UseNextDecl(ND).NextDecl;
+if (CurDecl->getDeclContext()->isFileContext())
+  if (const auto *TTP = dyn_cast(CurDecl))
+HandleDefaultTempArgIntoTempTempParam(TTP, Result);
+CurDecl = Response::UseNextDecl(CurDecl).NextDecl;
   }
 
-  if (!ND)
-CurDecl = Decl::castFromDeclContext(DC);
-
   while (!CurDecl->isFileContextDecl()) {
 Response R;
 if (const auto *VarTemplSpec =
diff --git a/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp 
b/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp
index 449b6232542e24..277935f6b3b2f0 100644
--- a/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp
+++ b/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp
@@ -59,3 +59,28 @@ struct Nothing {};
 
 // FIXME: Wait the standard to clarify the intent.
 template<> template<> Z S5::V;
+
+namespace GH57410 {
+
+template
+concept True = true;
+
+template
+concept False = false; // #False
+
+templa

[clang] [Clang] Correctly construct template arguments for file-scope template template parameters (PR #76811)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

I realized this still crashes the code with NTTP. I will look into this.

```cpp
template 
concept C = false;

template  struct S {};

template  typename T>
auto wow(T ts) {
  return ts.x;
}

int main() { return wow<3>(S{}); }
```

https://github.com/llvm/llvm-project/pull/76811
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Correctly construct template arguments for file-scope template template parameters (PR #76811)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

It is interesting to note that, if we exchange the position of two template 
parameters, i.e.

```cpp
template 
concept C = false;

template  struct S {};

template  typename T, unsigned N> // The 
template-template parameter now precedes the NTTP
int wow(T ts);

int main() { return wow(S{}); }
```

then the crash disappears.

https://github.com/llvm/llvm-project/pull/76811
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Correctly construct template arguments for file-scope template template parameters (PR #76811)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

Just get back to this again. So, I have explored the other case and found it 
particular to explicit template arguments. In fact, we could slightly revise 
the motivating case to

```cpp
template 
concept C = false;

template  typename T>
int wow(T ts);

template  struct S {};

int main() { return wow(S{}); }
```

with an *explicit* template argument `int`. This takes us to such the branch in 
`DeduceTemplateArguments`,

https://github.com/llvm/llvm-project/blob/e0c554ad87d18dcbfcb9b6485d0da800ae1338d1/clang/lib/Sema/SemaTemplateDeduction.cpp#L4222-L4232

which would deduce the remaining template arguments from the given 
`FunctionTemplate` and fall into our `getTemplateInstantiationArgs` at the end 
of the day. It is also interesting to notice that the given 
`TemplateTemplateParmDecl` originates from this `FunctionTemplate`, and the 
surrounding `DeclContext`s for these `Decl`s has already been *redirected* to 
its associated `FunctionDecl` at `AdoptTemplateParameterList`.

https://github.com/llvm/llvm-project/blob/59af659ee3c790d06cb5e2bf580e042547c24323/clang/lib/AST/DeclTemplate.cpp#L202-L206


Therefore, I think our assumption that the `TemplateTemplateParmDecl` always 
resides in the `TranslationUnitDecl` is probably wrong. And, it is indeed murky 
that the `DeclContext`s where these `Decl`s live depend on the callers to this 
function...

Removing these checks works for the code above and, fortunately, doesn't fail 
any tests.

@erichkeane WDYT?

https://github.com/llvm/llvm-project/pull/76811
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Correctly construct template arguments for file-scope template template parameters (PR #76811)

[Younan Zhang via cfe-commits]

@@ -345,15 +345,19 @@ MultiLevelTemplateArgumentList 
Sema::getTemplateInstantiationArgs(
 
   using namespace TemplateInstArgsHelpers;
   const Decl *CurDecl = ND;
+
+  if (!ND)

zyn0217 wrote:

> Can we also get a comment here why this makes sense to call 
> `Decl::castFromDeclContext(DC)` if `CurrDecl` is not present?

I think this has been explained in the comment above the function.
```cpp
/// \param DC In the event we don't HAVE a declaration yet, we instead provide
///  the decl context where it will be created.  In this case, the `Innermost`
///  should likely be provided.  If ND is non-null, this is ignored.
```

https://github.com/llvm/llvm-project/pull/76811
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Correctly construct template arguments for file-scope template template parameters (PR #76811)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/76811
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Correctly construct template arguments for file-scope template template parameters (PR #76811)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 updated 
https://github.com/llvm/llvm-project/pull/76811

>From 1164c705a8515d39bc9d4404e8523da8876d81cf Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Wed, 3 Jan 2024 19:33:01 +0800
Subject: [PATCH 1/2] [Clang] Correctly construct template arguments for
 file-scope template template parameters

This fixes the bug introduced by
https://github.com/llvm/llvm-project/commit/6db007a0654ed7a6ed5c3aa3b61a937c19a6bc6b.

We construct placeholder template arguments for template-template parameters to
avoid mismatching argument substitution since they have different depths
with their corresponding template arguments. In this case,

```cpp
template  class T> void foo(T);
```

T lies at the depth 0, and C lies at 1. The corresponding argument, of which
there is exactly one, int, is at depth 0. If we consider the
argument as the outermost one, then we would end up substituting 'int'
into the wrong parameter T.

We used to perform such placeholder construction during the context walk-up.
In the previous patch, we slipped through that inadvertently because we would
walk up to the parent, which is precisely a FileContext for template-template
parameters, after adding innermost arguments.

Besides, this patch moves the sanity check up to the context switch.
That way, we avoid dereferencing null pointers if ND is unspecified.

Closes https://github.com/llvm/llvm-project/issues/57410.
---
 clang/docs/ReleaseNotes.rst   |  3 +++
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 12 ++---
 .../temp/temp.arg/temp.arg.template/p3-2a.cpp | 25 +++
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0c8fec691bf3c9..7193d711333780 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -843,6 +843,9 @@ Bug Fixes to C++ Support
 - Fix crash when parsing nested requirement. Fixes:
   (`#73112 `_)
 
+- Fix a regression where clang forgets how to substitute into constraints on 
template-template
+  parameters. Fixes: (`#57410 
`_)
+
 Bug Fixes to AST Handling
 ^
 - Fixed an import failure of recursive friend class template.
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp 
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index df6b40999e645c..4420280efebb86 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -345,15 +345,19 @@ MultiLevelTemplateArgumentList 
Sema::getTemplateInstantiationArgs(
 
   using namespace TemplateInstArgsHelpers;
   const Decl *CurDecl = ND;
+
+  if (!ND)
+CurDecl = Decl::castFromDeclContext(DC);
+
   if (Innermost) {
 Result.addOuterTemplateArguments(const_cast(ND),
  Innermost->asArray(), Final);
-CurDecl = Response::UseNextDecl(ND).NextDecl;
+if (CurDecl->getDeclContext()->isFileContext())
+  if (const auto *TTP = dyn_cast(CurDecl))
+HandleDefaultTempArgIntoTempTempParam(TTP, Result);
+CurDecl = Response::UseNextDecl(CurDecl).NextDecl;
   }
 
-  if (!ND)
-CurDecl = Decl::castFromDeclContext(DC);
-
   while (!CurDecl->isFileContextDecl()) {
 Response R;
 if (const auto *VarTemplSpec =
diff --git a/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp 
b/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp
index 449b6232542e24..277935f6b3b2f0 100644
--- a/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp
+++ b/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp
@@ -59,3 +59,28 @@ struct Nothing {};
 
 // FIXME: Wait the standard to clarify the intent.
 template<> template<> Z S5::V;
+
+namespace GH57410 {
+
+template
+concept True = true;
+
+template
+concept False = false; // #False
+
+template typename Wrapper>
+using Test = Wrapper;
+
+template typename Wrapper> // #TTP-Wrapper
+using Test = Wrapper; // expected-error {{constraints not satisfied for 
template template parameter 'Wrapper' [with T = int]}}
+
+// expected-note@#TTP-Wrapper {{'int' does not satisfy 'False'}}
+// expected-note@#False {{evaluated to false}}
+
+template  typename T> // #TTP-foo
+void foo(T); // expected-error {{constraints not satisfied for template 
template parameter 'T' [with $0 = int]}}
+
+// expected-note@#TTP-foo {{'int' does not satisfy 'False'}}
+// expected-note@#False {{evaluated to false}}
+
+}

>From 8d5a7cdb39b0e64d350b8e4540738ef6eae7dc5c Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Fri, 5 Jan 2024 16:37:05 +0800
Subject: [PATCH 2/2] Address comments and poke the CI

---
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 22 ---
 .../temp/temp.arg/temp.arg.template/p3-2a.cpp | 12 ++
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp 
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 4420280ef

[clang] [Clang] Correctly construct template arguments for template template parameters (PR #76811)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/76811
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Correctly construct template arguments for template template parameters (PR #76811)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 edited 
https://github.com/llvm/llvm-project/pull/76811
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Correctly construct template arguments for template template parameters (PR #76811)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 updated 
https://github.com/llvm/llvm-project/pull/76811

>From 1164c705a8515d39bc9d4404e8523da8876d81cf Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Wed, 3 Jan 2024 19:33:01 +0800
Subject: [PATCH 1/3] [Clang] Correctly construct template arguments for
 file-scope template template parameters

This fixes the bug introduced by
https://github.com/llvm/llvm-project/commit/6db007a0654ed7a6ed5c3aa3b61a937c19a6bc6b.

We construct placeholder template arguments for template-template parameters to
avoid mismatching argument substitution since they have different depths
with their corresponding template arguments. In this case,

```cpp
template  class T> void foo(T);
```

T lies at the depth 0, and C lies at 1. The corresponding argument, of which
there is exactly one, int, is at depth 0. If we consider the
argument as the outermost one, then we would end up substituting 'int'
into the wrong parameter T.

We used to perform such placeholder construction during the context walk-up.
In the previous patch, we slipped through that inadvertently because we would
walk up to the parent, which is precisely a FileContext for template-template
parameters, after adding innermost arguments.

Besides, this patch moves the sanity check up to the context switch.
That way, we avoid dereferencing null pointers if ND is unspecified.

Closes https://github.com/llvm/llvm-project/issues/57410.
---
 clang/docs/ReleaseNotes.rst   |  3 +++
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 12 ++---
 .../temp/temp.arg/temp.arg.template/p3-2a.cpp | 25 +++
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0c8fec691bf3c9..7193d711333780 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -843,6 +843,9 @@ Bug Fixes to C++ Support
 - Fix crash when parsing nested requirement. Fixes:
   (`#73112 `_)
 
+- Fix a regression where clang forgets how to substitute into constraints on 
template-template
+  parameters. Fixes: (`#57410 
`_)
+
 Bug Fixes to AST Handling
 ^
 - Fixed an import failure of recursive friend class template.
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp 
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index df6b40999e645c..4420280efebb86 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -345,15 +345,19 @@ MultiLevelTemplateArgumentList 
Sema::getTemplateInstantiationArgs(
 
   using namespace TemplateInstArgsHelpers;
   const Decl *CurDecl = ND;
+
+  if (!ND)
+CurDecl = Decl::castFromDeclContext(DC);
+
   if (Innermost) {
 Result.addOuterTemplateArguments(const_cast(ND),
  Innermost->asArray(), Final);
-CurDecl = Response::UseNextDecl(ND).NextDecl;
+if (CurDecl->getDeclContext()->isFileContext())
+  if (const auto *TTP = dyn_cast(CurDecl))
+HandleDefaultTempArgIntoTempTempParam(TTP, Result);
+CurDecl = Response::UseNextDecl(CurDecl).NextDecl;
   }
 
-  if (!ND)
-CurDecl = Decl::castFromDeclContext(DC);
-
   while (!CurDecl->isFileContextDecl()) {
 Response R;
 if (const auto *VarTemplSpec =
diff --git a/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp 
b/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp
index 449b6232542e24..277935f6b3b2f0 100644
--- a/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp
+++ b/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp
@@ -59,3 +59,28 @@ struct Nothing {};
 
 // FIXME: Wait the standard to clarify the intent.
 template<> template<> Z S5::V;
+
+namespace GH57410 {
+
+template
+concept True = true;
+
+template
+concept False = false; // #False
+
+template typename Wrapper>
+using Test = Wrapper;
+
+template typename Wrapper> // #TTP-Wrapper
+using Test = Wrapper; // expected-error {{constraints not satisfied for 
template template parameter 'Wrapper' [with T = int]}}
+
+// expected-note@#TTP-Wrapper {{'int' does not satisfy 'False'}}
+// expected-note@#False {{evaluated to false}}
+
+template  typename T> // #TTP-foo
+void foo(T); // expected-error {{constraints not satisfied for template 
template parameter 'T' [with $0 = int]}}
+
+// expected-note@#TTP-foo {{'int' does not satisfy 'False'}}
+// expected-note@#False {{evaluated to false}}
+
+}

>From 8d5a7cdb39b0e64d350b8e4540738ef6eae7dc5c Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Fri, 5 Jan 2024 16:37:05 +0800
Subject: [PATCH 2/3] Address comments and poke the CI

---
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 22 ---
 .../temp/temp.arg/temp.arg.template/p3-2a.cpp | 12 ++
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp 
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 4420280ef

[clang] [Clang] Correctly construct template arguments for template template parameters (PR #76811)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 updated 
https://github.com/llvm/llvm-project/pull/76811

>From 1164c705a8515d39bc9d4404e8523da8876d81cf Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Wed, 3 Jan 2024 19:33:01 +0800
Subject: [PATCH 1/3] [Clang] Correctly construct template arguments for
 file-scope template template parameters

This fixes the bug introduced by
https://github.com/llvm/llvm-project/commit/6db007a0654ed7a6ed5c3aa3b61a937c19a6bc6b.

We construct placeholder template arguments for template-template parameters to
avoid mismatching argument substitution since they have different depths
with their corresponding template arguments. In this case,

```cpp
template  class T> void foo(T);
```

T lies at the depth 0, and C lies at 1. The corresponding argument, of which
there is exactly one, int, is at depth 0. If we consider the
argument as the outermost one, then we would end up substituting 'int'
into the wrong parameter T.

We used to perform such placeholder construction during the context walk-up.
In the previous patch, we slipped through that inadvertently because we would
walk up to the parent, which is precisely a FileContext for template-template
parameters, after adding innermost arguments.

Besides, this patch moves the sanity check up to the context switch.
That way, we avoid dereferencing null pointers if ND is unspecified.

Closes https://github.com/llvm/llvm-project/issues/57410.
---
 clang/docs/ReleaseNotes.rst   |  3 +++
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 12 ++---
 .../temp/temp.arg/temp.arg.template/p3-2a.cpp | 25 +++
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0c8fec691bf3c9..7193d711333780 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -843,6 +843,9 @@ Bug Fixes to C++ Support
 - Fix crash when parsing nested requirement. Fixes:
   (`#73112 `_)
 
+- Fix a regression where clang forgets how to substitute into constraints on 
template-template
+  parameters. Fixes: (`#57410 
`_)
+
 Bug Fixes to AST Handling
 ^
 - Fixed an import failure of recursive friend class template.
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp 
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index df6b40999e645c..4420280efebb86 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -345,15 +345,19 @@ MultiLevelTemplateArgumentList 
Sema::getTemplateInstantiationArgs(
 
   using namespace TemplateInstArgsHelpers;
   const Decl *CurDecl = ND;
+
+  if (!ND)
+CurDecl = Decl::castFromDeclContext(DC);
+
   if (Innermost) {
 Result.addOuterTemplateArguments(const_cast(ND),
  Innermost->asArray(), Final);
-CurDecl = Response::UseNextDecl(ND).NextDecl;
+if (CurDecl->getDeclContext()->isFileContext())
+  if (const auto *TTP = dyn_cast(CurDecl))
+HandleDefaultTempArgIntoTempTempParam(TTP, Result);
+CurDecl = Response::UseNextDecl(CurDecl).NextDecl;
   }
 
-  if (!ND)
-CurDecl = Decl::castFromDeclContext(DC);
-
   while (!CurDecl->isFileContextDecl()) {
 Response R;
 if (const auto *VarTemplSpec =
diff --git a/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp 
b/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp
index 449b6232542e24..277935f6b3b2f0 100644
--- a/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp
+++ b/clang/test/CXX/temp/temp.arg/temp.arg.template/p3-2a.cpp
@@ -59,3 +59,28 @@ struct Nothing {};
 
 // FIXME: Wait the standard to clarify the intent.
 template<> template<> Z S5::V;
+
+namespace GH57410 {
+
+template
+concept True = true;
+
+template
+concept False = false; // #False
+
+template typename Wrapper>
+using Test = Wrapper;
+
+template typename Wrapper> // #TTP-Wrapper
+using Test = Wrapper; // expected-error {{constraints not satisfied for 
template template parameter 'Wrapper' [with T = int]}}
+
+// expected-note@#TTP-Wrapper {{'int' does not satisfy 'False'}}
+// expected-note@#False {{evaluated to false}}
+
+template  typename T> // #TTP-foo
+void foo(T); // expected-error {{constraints not satisfied for template 
template parameter 'T' [with $0 = int]}}
+
+// expected-note@#TTP-foo {{'int' does not satisfy 'False'}}
+// expected-note@#False {{evaluated to false}}
+
+}

>From 8d5a7cdb39b0e64d350b8e4540738ef6eae7dc5c Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Fri, 5 Jan 2024 16:37:05 +0800
Subject: [PATCH 2/3] Address comments and poke the CI

---
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 22 ---
 .../temp/temp.arg/temp.arg.template/p3-2a.cpp | 12 ++
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp 
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 4420280ef

[clang] [clang] Correctly implement CWG 2672 (PR #75001)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 updated 
https://github.com/llvm/llvm-project/pull/75001

>From 8681b3c9f5e19b6ae977321d5d4154113273c2a0 Mon Sep 17 00:00:00 2001
From: Younan Zhang 
Date: Sun, 12 Nov 2023 13:21:03 +0800
Subject: [PATCH 1/2] [clang] Correctly implement CWG 2672

This is a follow-up patch for [D156993](https://reviews.llvm.org/D156993),
that marks only the lambda body as non-immediate context.

Fixes https://github.com/llvm/llvm-project/issues/71684
---
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 11 +---
 clang/lib/Sema/TreeTransform.h|  7 +
 clang/test/CXX/drs/dr26xx.cpp |  2 +-
 .../expr.prim.lambda/default-arguments.cpp|  6 ++--
 .../expr.prim/expr.prim.lambda/p11-1y.cpp |  2 --
 .../expr/expr.prim/expr.prim.lambda/p23.cpp   |  1 -
 .../expr/expr.prim/expr.prim.lambda/p4.cpp|  3 +-
 clang/test/CXX/temp/temp.deduct/p9.cpp|  8 ++
 clang/test/SemaCXX/cxx1y-init-captures.cpp|  8 +++---
 clang/test/SemaCXX/cxx1z-lambda-star-this.cpp |  4 +--
 clang/test/SemaCXX/lambda-expressions.cpp |  4 +--
 clang/test/SemaCXX/lambda-pack-expansion.cpp  |  1 -
 clang/test/SemaCXX/vartemplate-lambda.cpp |  1 -
 .../SemaCXX/warn-unused-lambda-capture.cpp| 28 +--
 .../SemaTemplate/instantiate-local-class.cpp  |  4 +--
 15 files changed, 37 insertions(+), 53 deletions(-)

diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp 
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index df6b40999e645c..88bd44f7d6934d 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -35,7 +35,6 @@
 #include "clang/Sema/Template.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/TemplateInstCallback.h"
-#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -1142,8 +1141,7 @@ std::optional 
Sema::isSFINAEContext() const {
 case CodeSynthesisContext::DeducedTemplateArgumentSubstitution:
   // We're either substituting explicitly-specified template arguments,
   // deduced template arguments. SFINAE applies unless we are in a lambda
-  // expression, see [temp.deduct]p9.
-  [[fallthrough]];
+  // body, see [temp.deduct]p9.
 case CodeSynthesisContext::ConstraintSubstitution:
 case CodeSynthesisContext::RequirementInstantiation:
 case CodeSynthesisContext::RequirementParameterInstantiation:
@@ -1444,13 +1442,6 @@ namespace {
   LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true);
   Sema::ConstraintEvalRAII RAII(*this);
 
-  Sema::CodeSynthesisContext C;
-  C.Kind = clang::Sema::CodeSynthesisContext::LambdaExpressionSubstitution;
-  C.PointOfInstantiation = E->getBeginLoc();
-  SemaRef.pushCodeSynthesisContext(C);
-  auto PopCtx =
-  llvm::make_scope_exit([this] { SemaRef.popCodeSynthesisContext(); });
-
   ExprResult Result = inherited::TransformLambdaExpr(E);
   if (Result.isInvalid())
 return Result;
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 1ad843d0bf4e0c..55e5c3c9dedc56 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -13648,10 +13648,17 @@ 
TreeTransform::TransformLambdaExpr(LambdaExpr *E) {
   getSema().PushExpressionEvaluationContext(
   Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
 
+  Sema::CodeSynthesisContext C;
+  C.Kind = clang::Sema::CodeSynthesisContext::LambdaExpressionSubstitution;
+  C.PointOfInstantiation = E->getBody()->getBeginLoc();
+  getSema().pushCodeSynthesisContext(C);
+
   // Instantiate the body of the lambda expression.
   StmtResult Body =
   Invalid ? StmtError() : getDerived().TransformLambdaBody(E, 
E->getBody());
 
+  getSema().popCodeSynthesisContext();
+
   // ActOnLambda* will pop the function scope for us.
   FuncScopeCleanup.disable();
 
diff --git a/clang/test/CXX/drs/dr26xx.cpp b/clang/test/CXX/drs/dr26xx.cpp
index dd4bb1ff6ae2e1..8a22dbeb98a3d5 100644
--- a/clang/test/CXX/drs/dr26xx.cpp
+++ b/clang/test/CXX/drs/dr26xx.cpp
@@ -211,7 +211,7 @@ void f(...);
 
 template 
 void bar(T) requires requires {
-   decltype([]() -> T {})::foo();
+   []() -> decltype(T::foo()) {};
 };
 void bar(...);
 
diff --git 
a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/default-arguments.cpp 
b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/default-arguments.cpp
index c5d08ec404a7c3..72265d77700aaf 100644
--- a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/default-arguments.cpp
+++ b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/default-arguments.cpp
@@ -35,8 +35,7 @@ struct NoDefaultCtor {
 template
 void defargs_in_template_unused(T t) {
   auto l1 = [](const T& value = T()) { };  // expected-error{{no matching 
constructor for initialization of 'NoDefaultCtor'}} \
-   // expected-note {{in instantiation 
of defa

[clang] [clang] Correctly implement CWG 2672 (PR #75001)

[Younan Zhang via cfe-commits]

zyn0217 wrote:

Thank you folks!

https://github.com/llvm/llvm-project/pull/75001
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits