Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't use inherited constant value via 'self' in enum case value #7821

Closed
wbars opened this issue Dec 23, 2021 · 8 comments
Closed

Can't use inherited constant value via 'self' in enum case value #7821

wbars opened this issue Dec 23, 2021 · 8 comments

Comments

@wbars
Copy link

wbars commented Dec 23, 2021

Description

The following code:

<?php

interface I {
    public const DEFAULT_VALUE = 'gambrinus';
}

enum Beer: string implements I {
    // case GAMBRINUS = I::DEFAULT_VALUE;
     case GAMBRINUS = self::DEFAULT_VALUE;
}
var_dump(Beer::GAMBRINUS);

Resulted in this output:

Fatal error: Enum case value must be compile-time evaluatable in /in/Oqm7f on line 9

But I expected this output instead:
No errors, because commented case GAMBRINUS = I::DEFAULT_VALUE produces no error

PHP Version

8.1.1

Operating System

No response

@wbars wbars changed the title Can't use inherited constant value via 'self' Can't use inherited constant value via 'self' in enum case value Dec 23, 2021
@wbars
Copy link
Author

wbars commented Dec 23, 2021

It seems I didn't fully understood compile-time semantics, can be closed.

@wbars wbars closed this as completed Dec 23, 2021
@cmb69
Copy link
Member

cmb69 commented Dec 23, 2021

Not so fast; https://3v4l.org/5sZes works.

@cmb69 cmb69 reopened this Dec 23, 2021
@nikic
Copy link
Member

nikic commented Dec 23, 2021

The problem here is that we compute a value to case table during compilation and all values must be known at that time. This happens pre-inheritance, so self::DEFAULT_VALUE is unknown at the time. This is unfortunate from a language design perspective, because what exactly can be evaluated during compilation and what can't is essentially an implementation detail (and may depend on whether opcache is used or not).

I think it should be possible to make this part of inheritance instead, which should allow us to evaluate all expressions. It would also delay some error reporting (about duplicate case values) to that point.

@iluuu1994 iluuu1994 self-assigned this Mar 10, 2022
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 10, 2022
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 11, 2022
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 11, 2022
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 11, 2022
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 25, 2022
@SerafimArts
Copy link
Contributor

SerafimArts commented Apr 21, 2022

Found another interesting feature-bug:

class Example
{
    final public const TEST = 42;
}

enum ExampleEnum: int
{
    case CASE = Example::TEST;
}
  1. If you run the code for the first time, then it works
  2. If you run the code again (PHP 8.1.4), an error occurs: Enum case value must be compile-time evaluatable

There is a suspicion that this is somehow related to opcache:

$ php test.php -dopcache.jit_buffer_size=0 -dopcache.enable=1 -dopcache.enable_cli=0
// - First execution 42
// - Second execution 42
// - Third execution 42

$ php test.php -dopcache.jit_buffer_size=0 -dopcache.enable=1 -dopcache.enable_cli=1
// - First execution 42
// - Second execution: Enum case value must be compile-time evaluatable
// - Third execution: Enum case value must be compile-time evaluatable

P.S.

PHP 8.1.4 (cli) (built: Apr  4 2022 13:30:33) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.4, Copyright (c) Zend Technologies
    with Zend OPcache v8.1.4, Copyright (c), by Zend Technologies

Does it make sense to try to reproduce the behavior in other versions of the language (8.1.3, 8.1.2, etc)?

@iluuu1994
Copy link
Member

iluuu1994 commented Apr 21, 2022

@SerafimArts Thanks! I think this should also be solved by #8190 but I'll make sure to add a test. You don't have to test it for older patches, not much has changed in the patches for enums. Edit: Although this qualifies as a bug. I'll see if I can fix that for 8.1.

@SerafimArts
Copy link
Contributor

Edit: Although this qualifies as a bug. I'll see if I can fix that for 8.1.

Yea. Let me move this information into a separate issue?

@iluuu1994
Copy link
Member

@SerafimArts Feel free to create a new issue.

@SerafimArts
Copy link
Contributor

@iluuu1994 Yes, I'm sorry, that's probably how it should have been done initially, so as not to confuse different situations.

iluuu1994 added a commit to iluuu1994/php-src that referenced this issue May 26, 2022
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue May 28, 2022
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue May 28, 2022
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Jun 9, 2022
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Jun 9, 2022
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Jun 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants