[Chapel Merge] Ignore unreachable code during resolution

Notifications Commits
1

Branch: refs/heads/main
Revision: a06df3b
Author: vasslitvinov
Link: Ignore unreachable code during resolution by vasslitvinov · Pull Request #20497 · chapel-lang/chapel · GitHub
Log Message:

Merge pull request #20497 from vasslitvinov/ignore-unreachable

Ignore unreachable code during resolution

Resolves or affects #14683 #15691 #18597 #20481

When a code block has a statement that interrupts the control flow, such as:

  • return
  • throw
  • call to a function that terminates the program, like halt()
  • an if whose both branches have this interruption property

then the remainder of the code block is discarded as unreachable.

The following function is now inferred to return void because its return statement is discarded:

proc testit() {
  if true {
halt();
  }
  return 5;
}

There are several places in our code base where we use code like the above, in several shapes and forms, to lead the compiler to infer the desired return type. This PR adjusts those cases that came up in testing. Ex. adjusted .bad for: test/errhandling/codePathCoverage/ifOrThrow2.chpl

The following function used to return a nilable, now returns a non-nilable class C:

proc makeC() {
  if true then
return new C();

  return new C()?;
}

If a throw or a halt is followed by a return, then the remainder of the block is handled normally.

Rationale: this exception supports the following code pattern. Test: test/sparse/CS/multiplication/correctness.chpl

// in parent class BaseSparseDomImpl
proc bulkAdd_help(....){
  halt("Helper function called on the BaseSparseDomImpl");

  return -1;
}

// in child class DefaultSparseDom
override proc bulkAdd_help(....){
  ....
  return actualAddCnt;  // return normally
}

The following would happen if we did not have this exception.

  • The parent's bulkAdd_help() does not contain any return statements for the compiler to consider, since "return -1" is discarded as unreachable. A function without return statements is inferred to return void.

  • This by itself is fine. The impact is merely that I cannot pass its result elsewhere, like writeln(bulkAdd_help(...)); is now an error.

  • The child's bulkAdd_help() is inferred to return int as usual.

  • There are cases where a variable whose static type is the parent (BaseSparseDomImpl) always contains a reference to a child (such as DefaultSparseDom). So the code author expects v.bulkAdd_help(...) to produce an integer. However the compiler goes to bulkAdd_help() on the static type of v, concludes that the call produces no value, and generates an error "cannot use void".

  • Non-executable declarations are retained, such as declarations of types and functions. Only the declarations of variables are removed.

  • Code inside task constructs is not considered. Otherwise we can lose some magic code that handles endCounts.

The change in lowerIterators fixes a compiler assertion failure when a follower iterator contains a throw as the last statement in its body, see test/errhandling/parallel/forall-iterator-throws-at-end.chpl . This change makes it equivalent to its sibling test forall-iterator-throws-follower.chpl .

While there: add a future test for #20496: test/statements/conditionals/param/nonparam-and-false.chpl.

  • Reflect this semantics in documentation.

  • Remove the Semantic Exception (see above), providing an alternative way to specify the return type of a function that never returns.

For example, have the compiler accept the following function and int as its return type -- because according to the analysis in this PR it does not return normally and int is the specified return type:

proc bulkAdd_help(): int {
  halt("Helper function called on the BaseSparseDomImpl");
}

r: @dlongnecke-cray

Modified Files:
A test/errhandling/parallel/forall-iterator-throws-at-end.chpl

A test/errhandling/parallel/forall-iterator-throws-at-end.good
A test/functions/resolution/ignore-unreachable.chpl
A test/functions/resolution/ignore-unreachable.good
A test/library/standard/Random/RandomStreamInterface/choiceTestLowbounded.chpl
A test/library/standard/Random/RandomStreamInterface/choiceTestLowbounded.good
A test/statements/conditionals/param/nonparam-and-false.bad
A test/statements/conditionals/param/nonparam-and-false.chpl
A test/statements/conditionals/param/nonparam-and-false.future
A test/statements/conditionals/param/nonparam-and-false.good
R test/users/weili/typefnbug.future
M compiler/resolution/functionResolution.cpp
M compiler/resolution/lowerIterators.cpp
M modules/dists/CyclicDist.chpl
M modules/standard/Random.chpl
M test/errhandling/codePathCoverage/ifOrThrow2.bad
M test/errhandling/codePathCoverage/ifOrThrow2.future
M test/library/standard/Random/RandomStreamInterface/choiceTestErrors.chpl
M test/types/records/intents/out-intent.good

Compare: https://github.com/chapel-lang/chapel/compare/9f25fed091cf...a06df3bb0e9a