Skip to content

Fix access to undefined stack index on ExpressionParser#67

Open
marcioAlmada wants to merge 17 commits intomasterfrom
fix/php74-expression-parser
Open

Fix access to undefined stack index on ExpressionParser#67
marcioAlmada wants to merge 17 commits intomasterfrom
fix/php74-expression-parser

Conversation

@marcioAlmada
Copy link
Copy Markdown
Owner

@marcioAlmada marcioAlmada commented Sep 30, 2019

Follow up to #64

marcioAlmada and others added 17 commits April 16, 2019 23:49
Ex.:

```
Error unpacking a non unpackable Ast node on `$(binary_xor?... {` at line 29 with context: [
    "^"
]

Hint: use a non ellipsis expansion as in `$(binary_xor ? {`
```

cc ircmaxell/php-compiler#29
```
$ast->{'some deep token'} = new Token(T_STRING, 'patch');
```

Ref: github.com//pull/59
From now on, the supported signature for expanders is:

```
function(Optional<Ast|TokenStream> $subject, Optional<Engine> $e) : Ast|TokenStream;
```

Support for nested Ast based expanders through Yay DSL also allowed:

Ex.:

```php
>> {
    $$(my_cheers_ast_expander(
        $$(my_hello_ast_expander(
            $(name_ast_node)
        ))
    ));
}
```

Given:

```php

function my_hello_ast_expander(\Yay\Ast $ast) : \Yay\Ast {
    return new \Yay\Ast($ast->label(), new Token(
        T_CONSTANT_ENCAPSED_STRING, "'Hello, {$ast->token()}. From Ast.'", $ast->token()->line()
    ));
}

function my_cheers_ast_expander(\Yay\Ast $ast) : \Yay\Ast {
    $str = str_replace("'", "", (string) $ast->token());

    return new \Yay\Ast($ast->label(), new Token(
        T_CONSTANT_ENCAPSED_STRING, "'{$str} Cheers!'", $ast->token()->line()
    ));
}

```

Thanks @assertchris for reproducing bugs and pushing tests.
- Augment error messages with their respective line and filename using the
same formating as the PHP engine itself. This allows IDEs to parse both
preprocessor and runtime error message the same way;
- Use a `YayPreprocessorError` instead of stdlib exceptions that can come
from any random user space code;
Using a stream wrapper requires url include enabled and this is
considered too unsafe for most setups. With that in mind I have
no reason to maintain this feature.

Other methods involving composer autoload trickery or xray
(https://github.com/marcioAlmada/xray) extension have been
considered preferable according to community usage history.
YAY!

Usage:
  yay
  yay <file>
  yay --macros=<macros>
  yay --macros=<macros> <file>
  yay -h | --help
  yay --version

Options:
  -h --help         Show this screen.
  --version         Show version.
  --macros=<macros> PHP glob pattern for macros to be loaded. Ex: my/macros/*.yay [default: '']

Examples:

```
# Process file:
yay input.php > output.php

# Process file, preload project macros:
> yay --macros="./project-macros/*.yay" input.php > output.php

# Process stdin:
> cat input.php | yay > output.php

# Process stdin, preload project macros:
> cat input.php | yay --macros="./project-macros/*.yay" > output.php
```
This allows the script to continue if xdebug can't be disabled
(most likely bc it isn't there in the first place)
@marcioAlmada
Copy link
Copy Markdown
Owner Author

1) Yay\ParserOptimizationTest::testExpressionParserWithGoodExpressions with data set
"Expression 446" ('3 << 3 >> 2 . 2', true)

The behavior of unparenthesized expressions containing both '.' and '>>'/'<<'
 will change in PHP 8:  '<<'/'>>' will take a higher precedence

There are still some precedence changes on some operators though, these last failures will require some thought. The ExpressionParser will have to be conditionally different regarding the PHP version.

BTW we should be thankfull these warnings were added, this is something nobody would notice without the engine warnings/notices.

@marcioAlmada marcioAlmada force-pushed the master branch 2 times, most recently from 1e82d5a to a647a0f Compare July 20, 2021 16:36
Comment thread Dockerfile
Comment on lines +1 to +9
FROM golang:1.10.1-alpine3.7 as builder

WORKDIR /go/src/nubank/authorizer

RUN apk --update add git openssh && \
rm -rf /var/lib/apt/lists/* && \
rm /var/cache/apk/*

RUN go get -u github.com/golang/dep/cmd/dep
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Attention: t his is the same Dockerfile that was reported with #71

@e1himself
Copy link
Copy Markdown

1) Yay\ParserOptimizationTest::testExpressionParserWithGoodExpressions with data set
"Expression 446" ('3 << 3 >> 2 . 2', true)

The behavior of unparenthesized expressions containing both '.' and '>>'/'<<'
will change in PHP 8: '<<'/'>>' will take a higher precedence

There are still some precedence changes on some operators though, these last failures will require some thought. The ExpressionParser will have to be conditionally different regarding the PHP version.

BTW we should be thankfull these warnings were added, this is something nobody would notice without the engine warnings/notices.

IMO you should not fix these in your tests. Even though these are rightful warnings from PHP that the code generated might be not what you want. Nevertheless, it's still 100% valid PHP syntax. The tool did exactly what it was told to. So I don't see a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants