llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28k stars 11.56k forks source link

Nested requires-expression in lambda causes ICE #61779

Open cjdb opened 1 year ago

cjdb commented 1 year ago

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)
llvmbot commented 1 year ago

@llvm/issue-subscribers-c-20

llvmbot commented 1 year ago

@llvm/issue-subscribers-clang-frontend

shafik commented 1 year ago

@erichkeane maybe related to https://github.com/llvm/llvm-project/issues/61776

erichkeane commented 1 year ago

@erichkeane maybe related to #61776

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

erichkeane commented 1 year ago

This one is likely https://github.com/llvm/llvm-project/issues/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 commented 1 year ago

@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 commented 6 months ago

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 commented 6 months ago

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 commented 5 months ago

Progress update: I finally make the code in this issue and https://github.com/llvm/llvm-project/issues/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 commented 4 months ago

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.

erichkeane commented 4 months ago

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 commented 4 months ago

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 commented 3 months ago

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 commented 3 months ago

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.

erichkeane commented 2 weeks ago

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.