[PATCH] D111814: Fix consteval crash when transforming 'this' expressions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:14839
 
   if (!IsInstantiation)
     PopDeclContext();
----------------
erichkeane wrote:
> so the only real change here is that `ExitFunctionBodyRAII` ends here, instead of the end of the function?  
> 
> I guess this SEEMS pretty innocuous, I don't know particularly what `PopFunctionScopeInfo` does well enough to know if this is going to cause problems, but I DO know at least the 14850+ doesn't need to have the function body in scope (and PopDeclContext seems like it doesn't do much?)...
> 
> I would think that anything after `PopDeclContext` would have a problem being in the function body anyway (that is, being in a 'function body' with a mismatched DeclContext seems wrong), so I'm reasonably convinced this is at least non-breaking.
> 
> so the only real change here is that ExitFunctionBodyRAII ends here, instead of the end of the function?

That's correct. I added the opening curly brace before the comments on the declaration of `ExitRAII`, and the close curly brace is on the line with the new comments added to it. Git decided this was the best diff it could come up with for all that re-indentation. :-/

> I would think that anything after PopDeclContext would have a problem being in the function body anyway (that is, being in a 'function body' with a mismatched DeclContext seems wrong), so I'm reasonably convinced this is at least non-breaking.

This matches my thinking -- just before we go to pop the declaration context or scope, *that* is when the function body has ended.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111814