squashing some bugs

src/script_opt/GenIDDefs.cc

@@ -62,8 +62,17 @@ TraversalCode GenIDDefs::PreStmt(const Stmt* s) {
             auto block = cr->Block();
 
             cr_active.push_back(confluence_blocks.size());
+            cr_return_seen.push_back(false);
             block->Traverse(this);
+
+            if ( cr_return_seen.back() )
+                // We encountered a return along the way.  curr_stmt is
+                // now set to the last statement in the block, so mark
+                // it with a return.
+                ReturnAt(curr_stmt);
+
             cr_active.pop_back();
+            cr_return_seen.pop_back();
 
             auto retvar = cr->RetVar();
             if ( retvar )
@@ -436,9 +445,11 @@ void GenIDDefs::ReturnAt(const Stmt* s) {
     // If we're right at a catch-return then we don't want to make the
     // identifier as encountering a scope-ending "return" here.  By avoiding
     // that, we're able to do optimization across catch-return blocks.
-    if ( cr_active.empty() || cr_active.back() != confluence_blocks.size() )
+    if ( cr_active.empty() || cr_active.back() != confluence_blocks.size() || cr_return_seen.back() )
         for ( auto id : modified_IDs.back() )
             id->GetOptInfo()->ReturnAt(s);
+    if ( ! cr_active.empty() )
+        cr_return_seen.back() = true;
 }
 
 void GenIDDefs::TrackID(const ID* id, const ExprPtr& e) {

src/script_opt/GenIDDefs.h

@@ -93,11 +93,15 @@ private:
 
     // Stack of confluence blocks corresponding to activate catch-return
     // statements.  We used to stop identifier definitions at these
-    // boundaries, but given there's no confluence (i.e., the body of the
-    // catch-return *will* execute), we can do broader optimization if we
-    // don't treat them as their own (new) confluence blocks.
+    // boundaries, but given there's limited confluence (i.e., the body of
+    // the catch-return *will* execute, up through its first return), we
+    // can do broader optimization if we don't treat them as their own
+    // (new) confluence blocks.
     std::vector<zeek_uint_t> cr_active;
 
+    // Parallel array that tracks whether a return has occurred.
+    std::vector<zeek_uint_t> cr_return_seen;
+
     // The following is parallel to confluence_blocks except
     // the front entry tracks identifiers at the outermost
     // (non-confluence) scope.  Thus, to index it for a given

src/script_opt/Reduce.cc

@@ -846,6 +846,15 @@ CSE_ValidityChecker::CSE_ValidityChecker(ProfileFuncs& _pfs, const std::vector<c
 }
 
 TraversalCode CSE_ValidityChecker::PreStmt(const Stmt* s) {
+    auto t = s->Tag();
+
+    if ( t == STMT_WHEN ) {
+        // These are too hard to analyze - they result in lambda calls
+        // that can affect aggregates, etc.
+        is_valid = false;
+        return TC_ABORTALL;
+    }
+
     if ( s->Tag() == STMT_ADD || s->Tag() == STMT_DELETE )
         in_aggr_mod_stmt = true;