Skip to content

Allow declare statement w/o parentheses #5808

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

Closed
wants to merge 1 commit into from

Conversation

brzuchal
Copy link
Contributor

@brzuchal brzuchal commented Jul 5, 2020

Implements language change allowing declare statement to skip parentheses.
This patch implements one of the proposed changes from Language constructs syntax changes RFC.

In example, allows:

declare strict_types = 1, ticks = 1;
declare encoding = 'ISO-8859-1';

@@ -362,6 +362,9 @@ top_statement:
| T_USE use_declarations ';' { $$ = $2; $$->attr = ZEND_SYMBOL_CLASS; }
| T_USE use_type use_declarations ';' { $$ = $3; $$->attr = $2; }
| T_CONST const_list ';' { $$ = $2; }
| T_DECLARE const_list ';'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be in statement: with the other declare definition? Otherwise there will be a weird discrepancy where parens must be used in inner statements.

// Works
if (true) {
    declare(ticks = 1);
}

// Parser error
if (true) {
    declare ticks = 1;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iluuu1994 Are we sure declare should be allowed inside any other statement?
By design declare has to be the very first statements.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By design declare has to be the very first statements.

That's not the case for the existing declare with parens though. The RFC doesn't mention handling the cases differently thus I think they should behave the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iluuu1994 that is indeed true. I'll modify it by introducing declare_list: '(' const_list ') | const_list; and referencing it from the statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iluuu1994 after thinking a while maybe we actually should provide it as a top_statement just like const is a top level statement and leave the expression syntax as is for now which allow it's depreciation in future along with ticks which have a really narrow use.

@nikic nikic added this to the PHP 8.0 milestone Jul 15, 2020
@brzuchal
Copy link
Contributor Author

I decided to withdraw the RFC

@brzuchal brzuchal closed this Jul 22, 2020
@nikic nikic removed this from the PHP 8.0 milestone Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants