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

[Pierre van Houtryve via Phabricator via cfe-commits]

Pierre-vh added a comment.

In D139713#3989071 <https://reviews.llvm.org/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 access the slot.
>
> If this is the case then maybe the checking in `EvaluateWithSubstitution(...)` needs to be more carefully done?
>
> I am not familiar with this code but I don't know if you analysis provides a convincing case the assert should be removed.

Indeed, this only happens when isValueDependent returns true.

I am also not familiar with the code, so I just decided to propose a quick fix to get the discussion started; I certainly don't mind changing the nature of the fix if we agree it should be fixed differently.
For instance, we could also make the "getParam" call faillible by adding some "tryGetParam" variant that doesn't have the assert, or by passing some optional boolean to indicate it's acceptable to have the key present in the map with a different version.

My initial reasoning was that if the assert can be broken by legitimate code, then it shouldn't be there


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139713/new/

https://reviews.llvm.org/D139713