[clang] [clang][bytecode] Fix crash in void functions returning non-void expr… (PR #176550)

[via cfe-commits]

================
@@ -5678,11 +5678,10 @@ bool Compiler<Emitter>::visitReturnStmt(const ReturnStmt *RS) {
       return this->emitRet(*ReturnType, RS);
     }
 
-    if (RE->getType()->isVoidType()) {
-      if (!this->visit(RE))
+    if (RE->containsErrors() || RE->getType()->isVoidType()) {
+      if (!this->discard(RE))
         return false;
     } else {
-      InitLinkScope<Emitter> ILS(this, InitLink::RVO());
       // RVO - construct the value in the return location.
----------------
Serosh-commits wrote:

@tbaederr
on the containsErrors() placement =  I originally hoisted it just to catch bad code early/everywhere, but you're totally right it really only matters for blocking the RVO path where things could go wrong. I'll move it down into the else block so it's only checked before visit/RVO .
On discard() =  I swapped visit() for discard() just to be safe about stack hygiene. I was worried visit() might leave a value hanging on the stack (like in the ternary case where a void function returns a non-void expr), whereas discard() explicitly evaluates side effects but guarantees a clean pop. That felt safer given the original RVOPtr assertion was stack-related. But if visit() + a manual pop (or whatever the usual pattern is) is preferred for consistency then i am happy to change it back 
On InitLinkScope = basically just skipped it for void functions because they don't do RVO at all, so setting up that scope felt redundant (and was what emitted the bad opcodes leading to the crash in the first place). Non-void cases keep it unchanged.
please need ur help on that i haven't pushed anything yet wanted to run this reasoning by you first. Does this make sense, or do you want me to adjust any of these before I update (revert discard, different guard for the scope) i will be happy to do that 

https://github.com/llvm/llvm-project/pull/176550