Suppress noreturn warning on when calling noreturn cdtors [GH #218]

[ilazaric]

Suppress noreturn_function_does_return warning if noreturn constructor or destructor is called [GH #218]

Previously we were testing for throw expressions and noreturn call expressions only,
extending to noreturn constructors and destructors as well.

Noting though, this warning suppression does not consider subexpressions,
it only operates on top-level expressions of expression statements.
This could be significantly improved by recursing over the sub-expressions,
while being careful around short-circuiting.
Moreover, init statements could use similar warning suppressions.

Half of src/statements.c diff is just whitespace, slightly cleaner diff -w:

diff --git a/src/statements.c b/src/statements.c
index 7d42a8c554..8bed826019 100644
--- a/src/statements.c
+++ b/src/statements.c
@@ -175,7 +175,7 @@ suppress warnings that might otherwise be issued later.
 
 static void check_reachability_following_expression(an_expr_node_ptr  node)
 /*
-If the indicated expression node represents a throw or a call of a function
+If the indicated expression node represents a throw or calls a function
 that may not return, update the current "reachability" to indicate that the
 code directly following the expression is (or may be) unreachable.
 */
@@ -196,8 +196,28 @@ code directly following the expression is (or may be) unreachable.
          (throw c, y)
        but it doesn't seem worth it. */
     set_unreachable(curr_reachability);
-  } else {
-    if (is_call_node(node)) {
+  } else if (node_is(node, enk_temp_init)) {
+    a_dynamic_init_ptr dip = node->variant.init.dynamic_init;
+    a_boolean          does_not_return = FALSE;
+    if (!dip) return;
+    if (dip->destructor && routine_does_not_return(dip->destructor)) {
+      does_not_return = TRUE;
+    }  /* if */
+    if (dyn_init_is(dip, dik_constructor)) {
+      a_routine_ptr rp = dip->variant.constructor.ptr;
+      if (rp && routine_does_not_return(rp)) {
+        does_not_return = TRUE;
+      }  /* if */
+    }  /* if */
+    if (does_not_return) {
+      /* The statement is invoking a constructor or destructor that
+         is marked as not returning.  Treat this like a lint notreached
+         comment -- i.e., as a hint to the compiler but not something we
+         know for sure. */
+      curr_reachability.reachable_considering_hints = FALSE;
+      curr_reachability.suppress_unreachable_warning = TRUE;
+    }
+  } else if (is_call_node(node)) {
     a_boolean   call_does_not_return = FALSE;
     a_type_ptr  routine_type;
     node = node->variant.operation.operands;
@@ -219,7 +239,6 @@ code directly following the expression is (or may be) unreachable.
       curr_reachability.suppress_unreachable_warning = TRUE;
     }  /* if */
   }  /* if */
-  }  /* if */
 }  /* check_reachability_following_expression */
 
 
@@ -5409,7 +5428,7 @@ the statement was preceded by the GNU C __extension__ keyword.
   }  /* if */
   if (expr != NULL) {
     sp->expr = expr;
-    /* If the expression is a throw expression or the call of a function that
+    /* If the expression is a throw expression or calls a function that
        is known not to return, the code following is unreachable. */
     check_reachability_following_expression(expr);
   } else {

[ilazaric]

This warning suppression is kinda interesting, I assume I compose with something that transforms cond ? E : E into just E, but haven't explored what specifically

int testTernaryUnconditionalNoreturn() {
  true ? NoReturn() : NoReturn();
}

[ilazaric]

@daveedvdv is something like this what you had in mind?

I ask because there are quite a few similar statements to the issue at hand,
but are not caught by this change:

// S is a class that throws on ctor or dtor
[[noreturn]] void a() { S{}; } // expression statement, PR affects it, erroneous warning removed
[[noreturn]] void b() { S s; } // init statement, PR affects expression statements only, erroneous warning persists
void c(const S&);
[[noreturn]] void d() { c(S{}); } // PR does not peer into sub-expressions, erroneous warning persists

Is this limited scope what you intended?
Don't mind trying for a more general solution covering sub-expressions and init-statements if so desired.

Also, this is not me pinging you for a review, I am not in any rush,
just wanted to ask on intended scope to make sure we are aligned.

[daveedvdv-nvidia]

We have a strict policy against "early returns".

[ilazaric]

Refactored a bit, joined up the suppression warnings from both blocks, killed the early return.

[daveedvdv-nvidia]

Best to use routine_from_function_expr to handle all the cases.

[ilazaric]

This implementation notices noreturn on function pointers as well,
I think switching to it would be a slight regression.

// imported/clang/c/Sema/attr-noreturn.sft.c
__attribute__((noreturn)) void f(__attribute__((noreturn)) void (*x)(void)) {
  x();
}

[daveedvdv-nvidia]

Don't mind trying for a more general solution covering sub-expressions and init-statements if so desired.

That would be awesome, but beware of traversal costs. Maybe start with just this, for now?