Skip to content

Nested requires-expression in lambda causes ICE #61779

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

Open
cjdb opened this issue Mar 28, 2023 · 15 comments
Open

Nested requires-expression in lambda causes ICE #61779

cjdb opened this issue Mar 28, 2023 · 15 comments
Assignees
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" concepts C++20 concepts confirmed Verified by a second party crash-on-valid lambda C++11 lambda expressions

Comments

@cjdb
Copy link
Contributor

cjdb commented Mar 28, 2023

The snippet below is derived from GCC's test concepts-lambda14.C causes an assertion failure. Unsure if it's a minimal repro, but it's as minimal as I could get it for now.

// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s

template<typename U>
void f() {
  []<typename T>(T){
    static_assert(requires { requires __is_same(T, int); }); // expected-error{{}}

    return 0;
  };
}

void g()
{
    f<int>();
}
clang++: /root/llvm-project/clang/lib/AST/ExprConstant.cpp:15330: bool clang::Expr::EvaluateAsConstantExpr(clang::Expr::EvalResult&, const clang::ASTContext&, clang::Expr::ConstantExprKind) const: Assertion `!isValueDependent() && "Expression evaluator can't be called on a dependent expression."' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: /opt/compiler-explorer/clang-assertions-trunk/bin/clang++ -gdwarf-4 -g -o /app/output.s -mllvm --x86-asm-syntax=intel -S --gcc-toolchain=/opt/compiler-explorer/gcc-snapshot -fcolor-diagnostics -fno-crash-diagnostics -std=c++20 <source>
1.	<eof> parser at end of file
2.	<source>:4:6: instantiating function definition 'f<int>'
 #0 0x000055959dac438f llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x3c7438f)
 #1 0x000055959dac20cc llvm::sys::CleanupOnSignal(unsigned long) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x3c720cc)
 #2 0x000055959da0f238 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0
 #3 0x00007fb100f43420 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x14420)
 #4 0x00007fb100a1000b raise (/lib/x86_64-linux-gnu/libc.so.6+0x4300b)
 #5 0x00007fb1009ef859 abort (/lib/x86_64-linux-gnu/libc.so.6+0x22859)
 #6 0x00007fb1009ef729 (/lib/x86_64-linux-gnu/libc.so.6+0x22729)
 #7 0x00007fb100a00fd6 (/lib/x86_64-linux-gnu/libc.so.6+0x33fd6)
 #8 0x00005595a130bd37 (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x74bbd37)
 #9 0x00005595a0450617 clang::ActionResult<clang::Expr*, true> calculateConstraintSatisfaction<calculateConstraintSatisfaction(clang::Sema&, clang::NamedDecl const*, clang::SourceLocation, clang::MultiLevelTemplateArgumentList const&, clang::Expr const*, clang::ConstraintSatisfaction&)::'lambda'(clang::Expr const*)>(clang::Sema&, clang::Expr const*, clang::ConstraintSatisfaction&, calculateConstraintSatisfaction(clang::Sema&, clang::NamedDecl const*, clang::SourceLocation, clang::MultiLevelTemplateArgumentList const&, clang::Expr const*, clang::ConstraintSatisfaction&)::'lambda'(clang::Expr const*)&&) SemaConcept.cpp:0:0
#10 0x00005595a0450ba2 CheckConstraintSatisfaction(clang::Sema&, clang::NamedDecl const*, llvm::ArrayRef<clang::Expr const*>, llvm::SmallVectorImpl<clang::Expr*>&, clang::MultiLevelTemplateArgumentList const&, clang::SourceRange, clang::ConstraintSatisfaction&) SemaConcept.cpp:0:0
#11 0x00005595a04510bf clang::Sema::CheckConstraintSatisfaction(clang::NamedDecl const*, llvm::ArrayRef<clang::Expr const*>, llvm::SmallVectorImpl<clang::Expr*>&, clang::MultiLevelTemplateArgumentList const&, clang::SourceRange, clang::ConstraintSatisfaction&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x66010bf)
#12 0x00005595a0cb8428 (anonymous namespace)::TemplateInstantiator::TransformNestedRequirement(clang::concepts::NestedRequirement*) SemaTemplateInstantiate.cpp:0:0
#13 0x00005595a0ce9196 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformRequiresExpr(clang::RequiresExpr*) SemaTemplateInstantiate.cpp:0:0
#14 0x00005595a0ce9d7e (anonymous namespace)::TemplateInstantiator::TransformRequiresExpr(clang::RequiresExpr*) SemaTemplateInstantiate.cpp:0:0
#15 0x00005595a0cbd193 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformExpr(clang::Expr*) SemaTemplateInstantiate.cpp:0:0
#16 0x00005595a0cf1a1c clang::Sema::SubstExpr(clang::Expr*, clang::MultiLevelTemplateArgumentList const&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6ea1a1c)
#17 0x00005595a0d03306 clang::TemplateDeclInstantiator::VisitStaticAssertDecl(clang::StaticAssertDecl*) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6eb3306)
#18 0x00005595a0d56098 void llvm::function_ref<void ()>::callback_fn<clang::Sema::SubstDecl(clang::Decl*, clang::DeclContext*, clang::MultiLevelTemplateArgumentList const&)::'lambda'()>(long) SemaTemplateInstantiateDecl.cpp:0:0
#19 0x00005595a02f6b05 clang::Sema::runWithSufficientStackSpace(clang::SourceLocation, llvm::function_ref<void ()>) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x64a6b05)
#20 0x00005595a0d04b21 clang::Sema::SubstDecl(clang::Decl*, clang::DeclContext*, clang::MultiLevelTemplateArgumentList const&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6eb4b21)
#21 0x00005595a0cb6a86 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformDeclStmt(clang::DeclStmt*) SemaTemplateInstantiate.cpp:0:0
#22 0x00005595a0cf7759 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformCompoundStmt(clang::CompoundStmt*, bool) SemaTemplateInstantiate.cpp:0:0
#23 0x00005595a0cbc8b6 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformLambdaExpr(clang::LambdaExpr*) SemaTemplateInstantiate.cpp:0:0
#24 0x00005595a0cbd3d9 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformExpr(clang::Expr*) SemaTemplateInstantiate.cpp:0:0
#25 0x00005595a0cf64ff clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformStmt(clang::Stmt*, clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::StmtDiscardKind) SemaTemplateInstantiate.cpp:0:0
#26 0x00005595a0cf7759 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformCompoundStmt(clang::CompoundStmt*, bool) SemaTemplateInstantiate.cpp:0:0
#27 0x00005595a0cfa85e clang::Sema::SubstStmt(clang::Stmt*, clang::MultiLevelTemplateArgumentList const&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6eaa85e)
#28 0x00005595a0d4b298 clang::Sema::InstantiateFunctionDefinition(clang::SourceLocation, clang::FunctionDecl*, bool, bool, bool) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6efb298)
#29 0x00005595a0d4967f clang::Sema::PerformPendingInstantiations(bool) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6ef967f)
#30 0x00005595a0316fa0 clang::Sema::ActOnEndOfTranslationUnitFragment(clang::Sema::TUFragmentKind) (.part.0) Sema.cpp:0:0
#31 0x00005595a031768a clang::Sema::ActOnEndOfTranslationUnit() (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x64c768a)
#32 0x00005595a01b2253 clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&, clang::Sema::ModuleImportState&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6362253)
#33 0x00005595a01a5dda clang::ParseAST(clang::Sema&, bool, bool) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6355dda)
#34 0x000055959ece4f78 clang::CodeGenAction::ExecuteAction() (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x4e94f78)
#35 0x000055959e5498a9 clang::FrontendAction::Execute() (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x46f98a9)
#36 0x000055959e4cddd6 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x467ddd6)
#37 0x000055959e62d537 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x47dd537)
#38 0x000055959b0425a6 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x11f25a6)
#39 0x000055959b03e3ca ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&, llvm::ToolContext const&) driver.cpp:0:0
#40 0x000055959e3360ad void llvm::function_ref<void ()>::callback_fn<clang::driver::CC1Command::Execute(llvm::ArrayRef<std::optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const::'lambda'()>(long) Job.cpp:0:0
#41 0x000055959da0f720 llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x3bbf720)
#42 0x000055959e33696f clang::driver::CC1Command::Execute(llvm::ArrayRef<std::optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const (.part.0) Job.cpp:0:0
#43 0x000055959e2fe1ac clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&, bool) const (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x44ae1ac)
#44 0x000055959e2fec4d clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&, bool) const (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x44aec4d)
#45 0x000055959e3068fd clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x44b68fd)
#46 0x000055959b040a50 clang_main(int, char**, llvm::ToolContext const&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x11f0a50)
#47 0x000055959af4c4c5 main (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x10fc4c5)
#48 0x00007fb1009f1083 __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24083)
#49 0x000055959b0390de _start (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x11e90de)
@cjdb cjdb added c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" concepts C++20 concepts crash Prefer [crash-on-valid] or [crash-on-invalid] labels Mar 28, 2023
@llvmbot
Copy link
Member

llvmbot commented Mar 28, 2023

@llvm/issue-subscribers-c-20

@llvmbot
Copy link
Member

llvmbot commented Mar 28, 2023

@llvm/issue-subscribers-clang-frontend

@shafik
Copy link
Collaborator

shafik commented Mar 29, 2023

@erichkeane maybe related to #61776

@shafik shafik added confirmed Verified by a second party crash-on-valid and removed crash Prefer [crash-on-valid] or [crash-on-invalid] labels Mar 29, 2023
@erichkeane
Copy link
Collaborator

erichkeane commented Mar 29, 2023

@erichkeane maybe related to #61776

Looks similar, but not the same cause apparently. Still crashes after the fix for #61776.

@erichkeane
Copy link
Collaborator

This one is likely #58872, the problem is we're instantiating the requires constraint before having a call to it. I had a WIP implementation here: https://reviews.llvm.org/D138148 but it wasnt taken over by @cschreib as anticipated.

@erichkeane
Copy link
Collaborator

@cschreib mentions in the other bug that he's unable to help out with that patch, so I/someone else need to grab it.

@LYP951018
Copy link
Contributor

I am trying to implement non-eager lambda instantiation, but I am not that familiar with clang yet, so progress will be relatively slow ;)

@erichkeane
Copy link
Collaborator

I am trying to implement non-eager lambda instantiation, but I am not that familiar with clang yet, so progress will be relatively slow ;)

That is great news! I did a quick start above, but never got to spend more than a few minutes on it. Let me know if you get stuck, and I'll help how I can.

@LYP951018
Copy link
Contributor

LYP951018 commented Apr 14, 2024

Progress update: I finally make the code in this issue and #58872 and https://reviews.llvm.org/D138148 (the test case provided by cschreib) compile ...
5e8f4f30289cd946edd41bc824db61fb
Next, I'll test nested generic lambda and more cases mixed with other language features (nontype template params, concepts, etc.)

@LYP951018
Copy link
Contributor

LYP951018 commented May 4, 2024

Handling the implicit capture of this in lambdas presents a challenge. Omitting the TransformLambdaBody process entirely would prevent us from accurately determining if the lambda requires an implicit capture of this.

#include <type_traits>

struct X {
  void f(int) { }
  static void f(double) { }
  
  int g() {
    auto L = [=](auto a) {
      return [](int i) { // expected-note {{explicitly capture 'this'}} expected-note {{while substituting into a lambda}}
        return [=](auto b) { // expected-note {{while substituting into a lambda}}
          f(decltype(a){}); //expected-error{{this}}
          int x = i;
        };
      };
    };
    L(3);
    return 0;
  }
};
int run = X{}.g();

My current implementation does not raise any compile error with this code (because the body is not instantiated). It will only flag an error when the body is instantiated (L(3)(0)(1)). Is this behavior compliant with the standard? Would this code be considered IFNDR?

If not, could we ONLY skip the instantiation of dependent if constexpr statements during TransformLambdaBody? Alternatively, are there other viable method? Do you have any thoughts on this @erichkeane ? Your input would be greatly appreciated.

@LYP951018 LYP951018 self-assigned this May 4, 2024
@erichkeane
Copy link
Collaborator

Handling the implicit capture of this in lambdas presents a challenge. Omitting the TransformLambdaBody process entirely would prevent us from accurately determining if the lambda requires an implicit capture of this.

#include <type_traits>

struct X {
  void f(int) { }
  static void f(double) { }
  
  int g() {
    auto L = [=](auto a) {
      return [](int i) { // expected-note {{explicitly capture 'this'}} expected-note {{while substituting into a lambda}}
        return [=](auto b) { // expected-note {{while substituting into a lambda}}
          f(decltype(a){}); //expected-error{{this}}
          int x = i;
        };
      };
    };
    L(3);
    return 0;
  }
};
int run = X{}.g();

My current implementation does not raise any compile error with this code (because the body is not instantiated). It will only flag an error when the body is instantiated (L(3)(0)(1)). Is this behavior compliant with the standard? Would this code be considered IFNDR?

If not, could we ONLY skip the instantiation of dependent if constexpr statements during TransformLambdaBody? Alternatively, are there other viable method? Do you have any thoughts on this @erichkeane ? Your input would be greatly appreciated.

I don't think we could do that. I THINK we still would like to analyze that to see if we can calculate the capture based on everything else.

In the example above, we SHOULD be able to diagnose that right away, 'a' clearly cannot be anything else (that is, its type is dependent, but not its lookup).

@LYP951018
Copy link
Contributor

I've encountered another example where the body needs to be instantiated to compute the capture set:

void bar(auto... v) {}

void foo(auto... v) {
    bar([&] { v; } ...);
}

foo(1, 1.0, 'c');

Currently, I plan to execute TransformLambdaBody as before, but the instantiation is only used to calculate the capture set, and I'll discard the instantiated body. During TransformLambdaBody, I will block all diagnostic triggered in TransformLambdaBody (similar to the implementation of SFINAE).

I'm not sure if this is feasible? Is there a better solution?

I've read through P0588R0 several times, and it mentions:

With this change in place, the set of captures of a lambda can be determined by a walk of the syntactic structure of the lambda-expression , which means that we do not need to instantiate the body of the lambda-expression within f(unique_ptr) in order to determine its capture set.

However, I can't figure out how to implement the computation of implicit this capture and the parameter pack expansion case added by this comment without instantiating the body under the existing architecture of Clang...

@LYP951018
Copy link
Contributor

I'm still working on the implmentation. Currently I bypass the instantiation of if constexpr while still processing the remainder of the body. This approach works for both the parameter pack scenario and the implicit this capture case (see comments above). However, this method struggles with parameter packs within constexpr if statements, as demonstrated in this example (gcc just iced.). Additionally, the current method leads to redundant diagnostics.

Quoting my previous comment:

I've read through P0588R0 several times, and it mentions:

With this change in place, the set of captures of a lambda can be determined by a walk of the syntactic structure of the lambda-expression , which means that we do not need to instantiate the body of the lambda-expression within f(unique_ptr) in order to determine its capture set.

However, I can't figure out how to implement the computation of implicit this capture and the parameter pack expansion case added by this comment without instantiating the body under the existing architecture of Clang...

I REALLY need some help here @erichkeane @zygoloid

@LYP951018
Copy link
Contributor

LYP951018 commented Jun 16, 2024

I experimented with another approach:

During TransformLambdaBody, I bypassed dependent if constexpr statements while still processing the remainder of the body.
Later, while instantiating the definition, I instantiated the previously skipped if constexpr statements separately (with different template args, scope, etc.).
This method effectively resolved the issue of redundant instantiations (redundant diagnostics). (I’m not sure if this approach will encounter other problems.)

However, I think I still require a visitor to address the parameter pack within the if constexpr case.

@EugeneZelenko EugeneZelenko added the lambda C++11 lambda expressions label Jun 16, 2024
@erichkeane
Copy link
Collaborator

Sorry for the delay, I was out for 3 months on bonding leave, so jsut catching up now. Sorry if my comment misses past context here...

I don't think just skipping the if constexpr is sufficient here, I think we end up needing to skip transforming the entire body unfortunately. Else we'll end up having spurious diagnostics on things that we shouldn't really be diagnosing 'yet'. Sure, they are things that would eventually fail and are probably covered under the "can never be instantiated" rule, but I wonder if we should avoid that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" concepts C++20 concepts confirmed Verified by a second party crash-on-valid lambda C++11 lambda expressions
Projects
Status: No status
Development

No branches or pull requests

6 participants