[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

This revision was automatically updated to reflect the committed changes. Closed by commit rL329671: [ExprConstant] Use an AST node and a version number as a key to create (authored by ahatanak, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.ll

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

ahatanak updated this revision to Diff 141788. ahatanak marked an inline comment as done. ahatanak set the repository for this revision to rC Clang. ahatanak added a comment. Use pair as the key. Repository: rC Clang https://reviews.llvm.org/D42776 Files: clang/include/clang/AST/APValue.h

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. I think this can be cleaned up a bit further, but I'm fine with that happening after the patch lands. Thanks! Comment at: lib/AST/ExprConstant.cpp:455-456 // values are

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

ahatanak updated this revision to Diff 139797. ahatanak added a comment. Assign an uninitialized APValue to a temporary instead of removing it from the temporary map when the temporary's lifetime has ended. https://reviews.llvm.org/D42776 Files: include/clang/AST/APValue.h lib/AST/APValue.

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

ahatanak added inline comments. Comment at: lib/AST/ExprConstant.cpp:5236 if (Frame) { - Result.set(VD, Frame->Index); + Result.set({VD, Frame->Index}); return true; rsmith wrote: > Hmm. We should be versioning local variables as well. Curre

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

ahatanak updated this revision to Diff 136124. ahatanak marked 7 inline comments as done. ahatanak added a comment. Address review comments. https://reviews.llvm.org/D42776 Files: include/clang/AST/APValue.h lib/AST/APValue.cpp lib/AST/ExprConstant.cpp test/SemaCXX/constant-expression-c

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

rsmith added a comment. Thank you, this looks like a great direction. As noted, there are a bunch of other cases that we should cover with this approach. I'm really happy about the number of related bugs we get to fix with this change. Comment at: lib/AST/ExprConstant.cpp:31

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

ahatanak added a comment. ping https://reviews.llvm.org/D42776 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

ahatanak added inline comments. Comment at: lib/AST/ExprConstant.cpp:589-598 +/// Keep track of the version of MTEs that are used by CXXDefaultArgExpr. +/// The version number is updated every time VisitCXXDefaultArgExpr is +/// called. +unsigned getDefaultArgNum(

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

ahatanak updated this revision to Diff 134560. ahatanak marked 3 inline comments as done. ahatanak added a comment. Address review comments. https://reviews.llvm.org/D42776 Files: include/clang/AST/APValue.h lib/AST/APValue.cpp lib/AST/ExprConstant.cpp test/SemaCXX/constexpr-default-arg

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

ahatanak added a comment. OK, I see. It's pretty easy to come up with an example. constexpr int foo1(int a = 12) { return a * a; } constexpr int foo2(int a = foo1()) { return a - 12; } https://reviews.llvm.org/D42776 ___ cfe-comm

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

ahatanak added inline comments. Comment at: lib/AST/ExprConstant.cpp:597 +} +void clearDefaultArgNum() { CurDefaultArgNum = 0; } +unsigned CurDefaultArgNum = 0, NextDefaultArgNum = 0; rsmith wrote: > This is wrong: these scopes can nest, so you can't

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

rsmith added inline comments. Comment at: lib/AST/APValue.cpp:27 +APValue::LValueBase Base; +bool IsOnePastTheEnd; CharUnits Offset; Move this to the end so it can share space with `IsNullPtr`. Comment at: lib/AST/ExprConstant.cpp:

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

ahatanak updated this revision to Diff 133947. ahatanak marked 2 inline comments as done. ahatanak added a comment. Address Erik's and Roman's review comments. https://reviews.llvm.org/D42776 Files: include/clang/AST/APValue.h lib/AST/APValue.cpp lib/AST/ExprConstant.cpp test/SemaCXX/co

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

ahatanak added inline comments. Comment at: lib/AST/ExprConstant.cpp:1165-1173 + auto LB = Temporaries.lower_bound(Key); + + // If an element with key Key is found, reset the value and return it. This + // can happen if Key is part of a default argument expression. + if (LB !

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

erik.pilkington added a comment. Hi Akira, thanks for working on this! Comment at: lib/AST/ExprConstant.cpp:1165-1173 + auto LB = Temporaries.lower_bound(Key); + + // If an element with key Key is found, reset the value and return it. This + // can happen if Key is part of a

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

lebedev.ri added inline comments. Comment at: test/SemaCXX/constexpr-default-arg.cpp:3 + +// expected-no-diagnostics + Down the line, it won't be obvious *what* this testcase is checking. At the very least wrap it into `namespace rdar_problem_36505742 {}` https

[PATCH] D42776: [Sema] Fix an assertion failure in constant expression evaluation of calls to functions with default arguments

ahatanak created this revision. ahatanak added a reviewer: rsmith. The assertion fails when a function with a default argument that materializes a temporary is called more than once in an expression. The assertion fails in CallStackFrame::createTemporary when it searches map Temporaries using th