Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Clang][Sema] Fix a crash in lambda instantiation #85565

Merged
merged 1 commit into from
Mar 20, 2024
Merged

Conversation

jcsxky
Copy link
Contributor

@jcsxky jcsxky commented Mar 17, 2024

Fix #85343
When build lambda expression in lambda instantiation, ThisType is required in Sema::CheckCXXThisCapture to build this capture. Set this type by import Sema::CXXThisScopeRAII and it will be used later in lambda expression transformation.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 17, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 17, 2024

@llvm/pr-subscribers-clang

Author: Qizhi Hu (jcsxky)

Changes

Fix #85343
When build lambda expression in lambda instantiation, ThisType is required in Sema::BuildCaptureInit. Set it in Sema::InstantiateFunctionDefinition when build capture of lambda and it will be used later in lambda expression transformation.


Full diff: https://github.com/llvm/llvm-project/pull/85565.diff

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+1)
  • (modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (+7)
  • (added) clang/test/Sema/PR85343.cpp (+22)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ba9de1ac98de08..26a87e9ab7b066 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -392,6 +392,7 @@ Bug Fixes to C++ Support
   Fixes (#GH84368).
 - Fixed a crash while checking constraints of a trailing requires-expression of a lambda, that the
   expression references to an entity declared outside of the lambda. (#GH64808)
+- Fix a crash in lambda instantiation that missing set ``ThisType`` when checking capture. Fixes (#GH85343).
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index dc972018e7b281..dcfb1798d69641 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -13,12 +13,14 @@
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTMutationListener.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclVisitor.h"
 #include "clang/AST/DependentDiagnostic.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/PrettyDeclStackTrace.h"
+#include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
@@ -29,6 +31,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"
 #include "clang/Sema/TemplateInstCallback.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/TimeProfiler.h"
 #include <optional>
 
@@ -5182,6 +5185,10 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
     // Enter the scope of this instantiation. We don't use
     // PushDeclContext because we don't have a scope.
     Sema::ContextRAII savedContext(*this, Function);
+    // We need ThisType in lambda instantiation.
+    Sema::CXXThisScopeRAII ThisScope(
+        *this, dyn_cast<CXXRecordDecl>(Function->getDeclContext()),
+        Qualifiers());
 
     FPFeaturesStateRAII SavedFPFeatures(*this);
     CurFPFeatures = FPOptions(getLangOpts());
diff --git a/clang/test/Sema/PR85343.cpp b/clang/test/Sema/PR85343.cpp
new file mode 100644
index 00000000000000..aa598a5df400bd
--- /dev/null
+++ b/clang/test/Sema/PR85343.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+// expected-no-diagnostics
+
+template <typename c> auto ab() -> c ;
+
+template <typename> struct e {};
+
+template <typename f> struct ac {
+  template <typename h> static e<decltype(ab<h>()(ab<int>))> i;
+  decltype(i<f>) j;
+};
+
+struct d {
+  template <typename f>
+  d(f) { 
+    ac<f> a;
+  }
+};
+struct a {
+  d b = [=](auto) { (void)[this] {}; };
+};
+void b() { new a; }
\ No newline at end of file

Copy link
Contributor

@Sirraide Sirraide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me apart from a few minor things.

clang/docs/ReleaseNotes.rst Outdated Show resolved Hide resolved
clang/lib/Sema/TreeTransform.h Outdated Show resolved Hide resolved
clang/lib/Sema/TreeTransform.h Outdated Show resolved Hide resolved
@jcsxky
Copy link
Contributor Author

jcsxky commented Mar 18, 2024

@Sirraide Very thankful for your comments and it really makes the description more clear and easy to be understood! I have updated this patch following your suggestion and please take another look.

@jcsxky jcsxky requested a review from Sirraide March 18, 2024 13:09
Copy link
Contributor

@Sirraide Sirraide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except that the comment could still be worded a bit better—especially since I had to take another look at how this here works exactly, but I believe that this is correct: CXXThisScopeRAII does nothing if the context that it is passed is null, and getFunctionLevelDeclContext stops if it finds a CXXRecordDecl, so this should be nonnull iff this lambda is in a class but outside a member function.

clang/lib/Sema/TreeTransform.h Outdated Show resolved Hide resolved
Copy link
Contributor

@Sirraide Sirraide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Sirraide
Copy link
Contributor

CI for the docs seems to be broken because of something clang-format related; the changes to the docs here are trivial, so we should just be able to ignore that.

@jcsxky jcsxky merged commit f5f3d5d into llvm:main Mar 20, 2024
4 of 5 checks passed
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
Fix llvm#85343
When build lambda expression in lambda instantiation, `ThisType` is
required in `Sema::CheckCXXThisCapture` to build `this` capture. Set
`this` type by import `Sema::CXXThisScopeRAII` and it will be used later
in lambda expression transformation.

Co-authored-by: huqizhi <836744285@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[18 regression] Assertion `!isNull() && "Cannot retrieve a NULL type pointer"' failed
3 participants