Support never types #6761

muglug · 2021-03-08T18:05:54Z

This provides a draft implementation of a never type, useful as a return type for functions that only ever throw or exit.

Zend/tests/return_types/noreturn_disallowed3.phpt

Zend/tests/return_types/noreturn_covariance.phpt

Zend/tests/return_types/noreturn_disallowed3.phpt

Zend/tests/return_types/noreturn_no_variance.phpt

Zend/zend_compile.c

Zend/zend_inheritance.c

iluuu1994 · 2021-03-08T18:23:45Z

You might also wanna add a terminating test (with exit) and a generator.

iluuu1994 · 2021-03-09T19:56:09Z

Another small test you could add:

--TEST--
noreturn in reflection
--FILE--
<?php

function foo(): noreturn {}

$noreturnType = (new ReflectionFunction('foo'))->getReturnType();

echo $noreturnType->getName() . "\n";
var_dump($noreturnType->isBuiltin());

?>
--EXPECT--
noreturn
bool(true)

Here's the fix for opcache:

Subject: [PATCH] Fix opcache for noreturn

---
 Zend/Optimizer/dce.c      | 1 +
 Zend/Optimizer/pass1.c    | 1 +
 Zend/Optimizer/zend_cfg.c | 2 ++
 3 files changed, 4 insertions(+)

diff --git a/Zend/Optimizer/dce.c b/Zend/Optimizer/dce.c
index 940f1b6ee2..85b32a7e10 100644
--- a/Zend/Optimizer/dce.c
+++ b/Zend/Optimizer/dce.c
@@ -155,6 +155,7 @@ static inline bool may_have_side_effects(
                case ZEND_TICKS:
                case ZEND_YIELD:
                case ZEND_YIELD_FROM:
+               case ZEND_VERIFY_NORETURN_TYPE:
                        /* Intrinsic side effects */
                        return 1;
                case ZEND_DO_FCALL:
diff --git a/Zend/Optimizer/pass1.c b/Zend/Optimizer/pass1.c
index 86774afef4..fc42002813 100644
--- a/Zend/Optimizer/pass1.c
+++ b/Zend/Optimizer/pass1.c
@@ -677,6 +677,7 @@ constant_binary_op:
                case ZEND_COALESCE:
                case ZEND_ASSERT_CHECK:
                case ZEND_JMP_NULL:
+               case ZEND_VERIFY_NORETURN_TYPE:
                        collect_constants = 0;
                        break;
                }
diff --git a/Zend/Optimizer/zend_cfg.c b/Zend/Optimizer/zend_cfg.c
index 973a93ef6c..9d62563693 100644
--- a/Zend/Optimizer/zend_cfg.c
+++ b/Zend/Optimizer/zend_cfg.c
@@ -301,6 +301,7 @@ ZEND_API int zend_build_cfg(zend_arena **arena, const zend_op_array *op_array, u
                        case ZEND_GENERATOR_RETURN:
                        case ZEND_EXIT:
                        case ZEND_MATCH_ERROR:
+                       case ZEND_VERIFY_NORETURN_TYPE:
                                if (i + 1 < op_array->last) {
                                        BB_START(i + 1);
                                }
@@ -514,6 +515,7 @@ ZEND_API int zend_build_cfg(zend_arena **arena, const zend_op_array *op_array, u
                        case ZEND_EXIT:
                        case ZEND_THROW:
                        case ZEND_MATCH_ERROR:
+                       case ZEND_VERIFY_NORETURN_TYPE:
                                break;
                        case ZEND_JMP:
                                block->successors_count = 1;
--

I'm far from an opcache expert, there might be more things missing. We'll have to ask someone more experienced.

tuqqu · 2021-03-10T19:47:20Z

Can a function that loops forever be marked as noreturn?
Hacklang, TypeScript and Rust (with the ! return type) support this.

function loop(): noreturn {
    while (true) {}
}

iluuu1994 · 2021-03-10T19:49:04Z

@tuqqu Yep, that should work fine. Should be specified in the RFC though. Hard to test in a test 😜

MaxSem · 2021-03-10T20:23:21Z

Zend/tests/return_types/noreturn_allowed.phpt

+
+try {
+    foo();
+} catch (Exception $e) {


Here and below: exception variable can be omitted now.

MaxSem · 2021-03-10T20:29:59Z

Zend/tests/return_types/noreturn_disallowed3.phpt

+foo();
+?>
+--EXPECTF--
+Fatal error: Uncaught TypeError: foo(): Nothing was expected to be returned in %s:%d


Hope you don't mind my suggestions:

Mention noreturn explicitly

Perhaps instead of "Nothing was expected to be returned" something like "Attempt to return from a noreturn function" should be clearer?

It's not a type error, but rather a logic one. The noreturn "type" is merely how functions are marked as never returning.

Perhaps instead of "Nothing was expected to be returned" something like "Attempt to return from a noreturn function" should be clearer?

This error is printed when the function ends without returning. So maybe something like "noreturn function was expected to throw, terminate or run infinitely". That's very explicit, maybe somebody has a better idea.

The noreturn "type" is merely how functions are marked as never returning.

noreturn is an actual type in PHPs type system. We could add a different error but I wouldn't say TypeError is wrong.

Zend/tests/return_types/noreturn_reflected.phpt

Zend/zend_inheritance.c

iluuu1994 · 2021-03-15T16:23:19Z

Zend/tests/return_types/noreturn_covariance.phpt

+
+    public function bar(): noreturn
+    {
+        throw new UnexpectedValueException('child');


Can you also add an inheritance test for a static method?

Actually meant one where noreturn is returned from a static method 🙂 I wasn't sure if there might be some issues with the == equality type check.

dstogov · 2021-04-01T10:26:23Z

Zend/zend_execute.c

+	zend_verify_type_error_common(
+		zf, arg_info, NULL, &fname, &fsep, &fclass, &need_msg, &given_msg);


what is the reason for this check?

As that's the only part you need, you can use get_function_or_method_name(zf) to fetch the function/method prefix for the error.

dstogov · 2021-04-01T10:27:26Z

Zend/zend_execute.c

@@ -1232,6 +1232,22 @@ ZEND_API ZEND_COLD void zend_verify_return_error(const zend_function *zf, zval *
 	zend_string_release(need_msg);
 }

+ZEND_API ZEND_COLD void zend_verify_noreturn_error(const zend_function *zf)


I would rename this into zend_noreturn_error() it always emits error.

dstogov · 2021-04-01T10:27:44Z

Zend/zend_vm_def.h

@@ -4237,6 +4237,13 @@ ZEND_VM_COLD_CONST_HANDLER(124, ZEND_VERIFY_RETURN_TYPE, CONST|TMP|VAR|UNUSED|CV
 	}
 }

+ZEND_VM_COLD_CONST_HANDLER(201, ZEND_VERIFY_NORETURN_TYPE, UNUSED, UNUSED)


I would rename this into ZEND_NORETURN or ZEND_NORETURN_ERROR.

Zend/zend_types.h

Zend/Optimizer/zend_dump.c

Zend/zend_inheritance.c

Zend/zend_execute.h

Zend/tests/return_types/never_disallowed3.phpt

nikic

Looks fine. I'm missing a test for generator with never return type.

Zend/zend_compile.c

nikic · 2021-04-16T10:06:23Z

Zend/zend_execute.c

+	zend_verify_type_error_common(
+		zf, arg_info, NULL, &fname, &fsep, &fclass, &need_msg, &given_msg);


As that's the only part you need, you can use get_function_or_method_name(zf) to fetch the function/method prefix for the error.

Zend/zend_vm_def.h

ext/tokenizer/tokenizer_data.c

muglug · 2021-04-16T16:38:27Z

@nikic I've added the test for a generator function returning never

TysonAndre · 2021-04-17T13:45:45Z

ext/tokenizer/tokenizer_data.c

@@ -16,7 +16,7 @@

 /*
   DO NOT EDIT THIS FILE!
-   This file is generated using tokenizer_data_gen.php
+   This file is generated using tokenizer_data_gen.sh


The commits in this PR seems like they should be rebased and squashed - the changes to tokenizer_data.c ended up being unnecessary?

> **Never type** > > A new`return only type `never` has been added. This indicates that a function either `exit()`, throws an exception, or doesn't terminate. Includes unit tests. Refs: * https://wiki.php.net/rfc/noreturn_type * https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.core.never-type * php/php-src#6761 * php/php-src@6cd0b48

muglug added 4 commits March 6, 2021 22:42

Support noreturn types

d5d2d03

Add more tests

d24d458

Attempt opcode adjustment

a763287

Add covariance checks

4dd3c32

iluuu1994 reviewed Mar 8, 2021

View reviewed changes

muglug added 6 commits March 8, 2021 22:35

Fix test formatting

db5b67c

Do straightforward noreturn comparison

0395486

Do straightforward noreturn comparison

f59b56a

Fix test description

03a9ede

Make exception more precise

984de02

Don’t emit return type check if noreturn is given

3ec058f

muglug added 2 commits March 10, 2021 10:13

Add a reflection test

38449b7

Incorporate opcache suggestions from @iluuu1994

e3033ae

muglug marked this pull request as ready for review March 10, 2021 17:37

MaxSem reviewed Mar 10, 2021

View reviewed changes

Add combinations of return and throw as tests

56ae52e

alfredbez mentioned this pull request Mar 11, 2021

Propose never type php-fig/fig-standards#1191

Merged

Girgias reviewed Mar 14, 2021

View reviewed changes

Zend/tests/return_types/noreturn_reflected.phpt Outdated Show resolved Hide resolved

nikic reviewed Mar 15, 2021

View reviewed changes

Zend/zend_inheritance.c Outdated Show resolved Hide resolved

Add specific return-by-ref test and remove unnecessary check

6d3a76d

iluuu1994 reviewed Mar 19, 2021

View reviewed changes

muglug added 2 commits March 19, 2021 12:14

Add another test for noreturn as the bottom type

1e7ce1f

Add a test for __toString noreturn

8763f7f

dstogov reviewed Apr 1, 2021

View reviewed changes

carusogabriel added the RFC label Apr 7, 2021

Merge remote-tracking branch 'upstream/master' into support-noreturn

30f2013

Use never instead of noreturn

1c05283

azjezz reviewed Apr 13, 2021

View reviewed changes

Zend/Optimizer/zend_dump.c Outdated Show resolved Hide resolved

azjezz reviewed Apr 13, 2021

View reviewed changes

Zend/zend_inheritance.c Outdated Show resolved Hide resolved

azjezz reviewed Apr 13, 2021

View reviewed changes

Zend/zend_execute.h Outdated Show resolved Hide resolved

muglug changed the title ~~Support noreturn types~~ Support never types Apr 14, 2021

Migrate more to never

fd5bfa3

azjezz reviewed Apr 15, 2021

View reviewed changes

Zend/tests/return_types/never_disallowed3.phpt Outdated Show resolved Hide resolved

muglug added 2 commits April 15, 2021 18:53

Update message for never-returning function

47af885

Fix spacing

b087460

nikic reviewed Apr 16, 2021

View reviewed changes

muglug added 3 commits April 16, 2021 11:02

Improve some language and other PR changes

f59ec6d

Merge remote-tracking branch 'upstream/master' into support-noreturn

f16cb38

Add return type for generator returning never

1b68ae6

muglug added 2 commits April 16, 2021 12:46

Simplify zend_verify_never_error

0def0c9

Release memory

044207b

TysonAndre reviewed Apr 17, 2021

View reviewed changes

thekid mentioned this pull request Apr 17, 2021

Allow never return type xp-framework/compiler#109

Closed

TysonAndre mentioned this pull request Apr 18, 2021

Release ast version 1.0.11 nikic/php-ast#203

Merged

nikic approved these changes Apr 19, 2021

View reviewed changes

nikic closed this in 6cd0b48 Apr 19, 2021

zonuexe mentioned this pull request Apr 19, 2021

Add never native type for PHP 8.1 emacs-php/php-mode#660

Merged

muglug deleted the support-noreturn branch April 19, 2021 12:29

JanTvrdik mentioned this pull request Jul 2, 2021

Methods like sendResponse() should contain never-return typehint nette/application#290

Closed

jrfnl mentioned this pull request Dec 30, 2022

PHP 8.1 | NewReturnTypeDeclarations: support never and verify standalone types PHPCompatibility/PHPCompatibility#1458

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support never types #6761

Support never types #6761

muglug commented Mar 8, 2021 •

edited

Loading

iluuu1994 commented Mar 8, 2021

iluuu1994 commented Mar 9, 2021

tuqqu commented Mar 10, 2021

iluuu1994 commented Mar 10, 2021 •

edited

Loading

MaxSem Mar 10, 2021

MaxSem Mar 10, 2021

iluuu1994 Mar 10, 2021

iluuu1994 Mar 15, 2021

muglug Mar 19, 2021

iluuu1994 Mar 19, 2021

dstogov Apr 1, 2021

nikic Apr 16, 2021

dstogov Apr 1, 2021

dstogov Apr 1, 2021

nikic left a comment

nikic Apr 16, 2021

muglug commented Apr 16, 2021

TysonAndre Apr 17, 2021

		zend_verify_type_error_common(
		zf, arg_info, NULL, &fname, &fsep, &fclass, &need_msg, &given_msg);

Support never types #6761

Support never types #6761

Conversation

muglug commented Mar 8, 2021 • edited Loading

iluuu1994 commented Mar 8, 2021

iluuu1994 commented Mar 9, 2021

tuqqu commented Mar 10, 2021

iluuu1994 commented Mar 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muglug commented Apr 16, 2021

Choose a reason for hiding this comment

muglug commented Mar 8, 2021 •

edited

Loading

iluuu1994 commented Mar 10, 2021 •

edited

Loading