dmd icon indicating copy to clipboard operation
dmd copied to clipboard

Infer noreturn for auto functions

Open MoonlightSentinel opened this issue 4 years ago • 5 comments

Infer noreturn whenever the function body is never exited normally.

Also added a bunch of tests from the DIP that really should've been added in the initial PR's... (because many of them don't work)


Required changes:

  • [x] #13170 (fixes Vibe-D tests)
  • [x] dlang/druntime#3581
  • [x] dlang/phobos#8264
  • [x] dlang/phobos#8265
  • [x] dlang/phobos#8266
  • [x] dlang/phobos#8270
  • [x] dlang/phobos#8273
  • [x] libmir/mir-core#49
  • [ ] vibe-d/vibe-core#299

Related changes:

  • [x] #13135
  • [x] #13168
  • [x] dlang/phobos#8276

MoonlightSentinel avatar May 21 '21 14:05 MoonlightSentinel

Thanks for your pull request and interest in making D better, @MoonlightSentinel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#12553"

dlang-bot avatar May 21 '21 14:05 dlang-bot

This PR currently has a negative implication for callbacks as seen in the druntime failure:

void fun(void function() f);
void fun(void delegate() d);

fun(() { throw new Exception(""); });

The call above is now ambiguous because both overloads are considered as equal (MATCH.convert due to noreturn => void).

MoonlightSentinel avatar May 21 '21 14:05 MoonlightSentinel

Resolved the ambiguity by upping the matching level for noreturn => void to MATCH.constant. This is valid because both neither noreturn nor void represent actual values and hence are more closely related than other types.

Another possible approach would be to adapt the matching code for function / delegate literals, allthough that wouldn't be as encapsulated as the current approach.


This currently fails because of the dreaded statement not reachable warning in templated code, e.g. for the non-error path (return null) in collectException.

MoonlightSentinel avatar Oct 05 '21 19:10 MoonlightSentinel

Resolved the ambiguity by upping the matching level for noreturn => void to MATCH.constant. This is valid because both neither noreturn nor void represent actual values and hence are more closely related than other types.

Really not a fan of putting special-case hacks like this in the type system. This is the kind of decision that can seem like the lesser of two evils at the time, but that future D programmers will end up cursing us for.

The correct solution is to disambiguate the call in druntime by giving the function literal an explicit return type.

(Arguably, the actual correct solution is to implement implicit conversion of function pointers to delegate pointers, which would eliminate the need for separate overloads in the first place—but that's out of scope for this discussion.)

pbackus avatar Oct 05 '21 20:10 pbackus

@MoonlightSentinel What is the state of this draft?

RazvanN7 avatar Apr 26 '22 08:04 RazvanN7