Ensure variable declarations at the end of try blocks are cleaned up (#…

…25919)

Closes #25548, in which
`try! whatever()` caused the result of `whatever()` to not be cleaned
up, even if no exception occurred.

To see why this happens, consider the following (simplified) code
desugaring `try! expr`:

```Chapel
var x1 = ... 
var x2 = ...
var x3 = ...
if (error) {
  goto error handling;
}
```

`x3 = …` is presumably the call that creates the error, so `x3` may be
uninitialized at the time we do `if (error)`. When traversing this code
linearly and inserting autodestroys, to avoid processing `x3` (which may
be uninit'ed), we go right into the `if (error)` just before we visit
`x3` (going out of order). This way, when we insert auto-destroys for
error handling (which happens to auto-destroy all variables in scope,
since we are unwinding), we don’t auto-destroy `x3`, which may be
uninit’ed.

However, we use the same code to do this early visit into `if (error)`
as we do for any other statement. And other logic in that code says
“well if this statement is the last mention of any variables, insert
auto-destroys”. Thus, we insert auto-destroys while doing this early
processing of `if (error)`, and mark all variables as having been
deinited… except that we haven’t marked `x3` as inited yet (previous
paragraph), so it doesn’t get an auto-destroy, so it leaks. We mark it
as deinited without having inserted the deinitialization code.

interestingly we visit `if (error)` again, normally, after `x3`, and it
tries to do “insert auto-destroys” as well, but it’s a nop-op since
they’ve already been inserted and all the variables have been marked
already-uninitialized. This PR just makes the early `if (error)` not do
the cleanup (since it will miss `x3`), and lets that cleanup fall
through to the non-early if (error) handling. The early `if (error)`
still inserts auto-destroys for unwinding (as one would expect).

Reviewed by @jabraham17 -- thanks!

## Testing
- [x] paratest
- [x] paratest (memleaks)

compiler/resolution/addAutoDestroyCalls.cpp

@@ -176,6 +176,7 @@ static Expr* walkBlockStmt(FnSymbol*         fn,
                            LabelSymbol*      retLabel,
                            bool              isDeadCode,
                            bool              inScopelessBlock,
+                           bool              isEarlyVisitForGotoError,
                            Expr*             stmt,
                            std::set<VarSymbol*>& ignoredVariables,
                            LastMentionMap&   lmm) {
@@ -207,7 +208,7 @@ static Expr* walkBlockStmt(FnSymbol*         fn,
 
     // Consider the catch blocks now
     for (Expr* cur = stmt->next; cur != NULL; cur = cur->next) {
-      cur = walkBlockStmt(fn, scope, retLabel, false, false, cur,
+      cur = walkBlockStmt(fn, scope, retLabel, false, false, false, cur,
                           ignoredVariables, lmm);
       ret = cur;
     }
@@ -256,7 +257,7 @@ static Expr* walkBlockStmt(FnSymbol*         fn,
       if (isCheckErrorStmt(stmt->next)) {
         // Visit the check-error block now - do not consider
         // the variables initialized when running that check-error block.
-        ret = walkBlockStmt(fn, scope, retLabel, false, false, stmt->next,
+        ret = walkBlockStmt(fn, scope, retLabel, false, false, true, stmt->next,
                             ignoredVariables, lmm);
       }
 
@@ -370,7 +371,7 @@ static Expr* walkBlockStmt(FnSymbol*         fn,
     }
   }
 
-  if (isDeadCode == false) {
+  if (isDeadCode == false && isEarlyVisitForGotoError == false) {
     // Destroy the variable after this statement if it's the last mention
     // Since this adds the destroy immediately after this statement,
     // it ends up destroying multiple variables to be destroyed here
@@ -400,7 +401,7 @@ static void walkBlockScopelessBlock(AutoDestroyScope& scope,
                                     std::set<VarSymbol*>& ignoredVariables,
                                     LastMentionMap&   lmm) {
   for (Expr* stmt = block->body.first(); stmt != NULL; stmt = stmt->next) {
-    stmt = walkBlockStmt(fn, scope, retLabel, isDeadCode, true, stmt,
+    stmt = walkBlockStmt(fn, scope, retLabel, isDeadCode, true, false, stmt,
                          ignoredVariables, lmm);
   }
 }
@@ -556,7 +557,7 @@ static void walkBlockWithScope(AutoDestroyScope& scope,
     //
     // Handle the current statement
     //
-    stmt = walkBlockStmt(fn, scope, retLabel, isDeadCode, false, stmt,
+    stmt = walkBlockStmt(fn, scope, retLabel, isDeadCode, false, false, stmt,
                          ignoredVariables, lmm);
 
     //

test/errhandling/stmt-try-memory-leak.chpl

@@ -0,0 +1,18 @@
+record R {
+  proc deinit() do writeln("deinit");
+}
+class C {
+  proc deinit() do writeln("deinit");
+}
+
+config type T = R;
+
+proc get() : T throws {
+  var a = new T();
+  return a;
+}
+
+proc main() {
+  var a = try! get();
+  try! get();
+}

test/errhandling/stmt-try-memory-leak.compopts

@@ -0,0 +1,3 @@
+-sT=R
+-sT='owned C'
+-sT='shared C'

test/errhandling/stmt-try-memory-leak.good

@@ -0,0 +1,2 @@
+deinit
+deinit