[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd6acd0196b33: [Sema] Fix crash when evaluating nested call with value-dependent arg (authored by Pierre-vh). Repository: rG LLVM Github Monorepo

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

ahatanak accepted this revision. ahatanak added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139713/new/ https://reviews.llvm.org/D139713

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

Pierre-vh updated this revision to Diff 486183. Pierre-vh added a comment. Remove assert Please review @ahatanak Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139713/new/ https://reviews.llvm.org/D139713 Files: clang/lib/AST/ExprConstant.cpp

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

ahatanak added a comment. I read the patches and review comments in https://reviews.llvm.org/D42776, but I don't remember why I added that assert. Maybe I was just trying to ensure the version number passed to `getTemporary` was the one that was used to create the temporary. For example, if a

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

Pierre-vh updated this revision to Diff 482384. Pierre-vh added a comment. Put the assert back in, use alternative fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139713/new/ https://reviews.llvm.org/D139713 Files:

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

shafik added a comment. In D139713#3989709 , @Pierre-vh wrote: > In D139713#3989071 , @shafik wrote: > >> If I am reading the code correctly it looks like if the call to >> `(*I)->isValueDependent()` is true

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

Pierre-vh added a comment. In D139713#3989071 , @shafik wrote: > If I am reading the code correctly it looks like if the call to > `(*I)->isValueDependent()` is true then the temporary will not be created and > therefore we should not be attempting to

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

shafik added a comment. If I am reading the code correctly it looks like if the call to `(*I)->isValueDependent()` is true then the temporary will not be created and therefore we should not be attempting to access the slot. If this is the case then maybe the checking in

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

Pierre-vh added a comment. I think the right choice is to remove the assert as it's an invariant that can be broken in some cases, so I don't feel like the assert is worth it anymore. Alternatively I could add something to bypass the assert in that specific call to getParamSlot for

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

Pierre-vh updated this revision to Diff 481626. Pierre-vh added a comment. Missing newline at EOF Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139713/new/ https://reviews.llvm.org/D139713 Files: clang/lib/AST/ExprConstant.cpp

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

Pierre-vh created this revision. Pierre-vh added reviewers: ahatanak, rsmith. Herald added a project: All. Pierre-vh requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fix an edge case `ExprConstant.cpp`'s `EvaluateWithSubstitution` when