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

Bug in nested loops or matching a list with trailing delimiter? #30

Closed
SerafimArts opened this issue Sep 13, 2016 · 4 comments
Closed

Comments

@SerafimArts
Copy link
Contributor

A little experiment for simple annotations implementation:

input:

@Any(a = 66)
@Some(a = 23, b = 42)
class TestClass {}

macro:

macro ·global ·unsafe {
    @·ns()·class(
        ·ls (
            ·chain (T_STRING·field, ·token('='), ·label()·value),
            ·token(',')
        )
        ·fields
    )
    class T_STRING·class_name
} >> {
    new class(new \ReflectionClass(T_STRING·class_name::class)) extends ·class {
        public function __construct(\ReflectionClass $context) {
            $fields = [];
            ·fields ··· {
                $this->T_STRING·field = $fields[··stringify(T_STRING·field)] = ·value;
            }
            parent::__construct($fields, $context);
        }
    }
    class T_STRING·class_name
}

output:

@Any(a = 66) // <- this is bug
new class(new \ReflectionClass(TestClass::class)) extends Some {
  public function __construct(\ReflectionClass $context) {
    $fields = [];
    $this->a = $fields['a'] = 23;
    $this->b = $fields['b'] = 42;
    parent::__construct($fields, $context);
  }
}
class TestClass {}

Try to fix bug on line 1:

macro ·global ·unsafe {
    ·ls ( // <- Add one more loop
        ·chain (
            ·token('@'), ·ns()·class,
            ·token('('),
                ·ls (
                    ·chain (T_STRING·field, ·token('='), ·label()·value),
                    ·token(',')
                ) ·annotation_arguments,
            ·token(')')
        ),
        ·token(T_WHITESPACE)
    ) ·annotations

    class T_STRING·class_name
} >> {
    ·annotations ··· {
        new class(new \ReflectionClass(T_STRING·class_name::class)) extends ·class
        {
            public function __construct(\ReflectionClass $context)
            {
                $fields = [];
                ·annotation_arguments ··· {
                    $this->T_STRING·field = $fields[··stringify(T_STRING·field)] = ·value;
                }
                parent::__construct($fields, $context);
            }
        }
    }
    class T_STRING·class_name
}

New ouput:

@Any(a = 66)
new class(new \ReflectionClass(TestClass::class)) extends Some
        {
            public function __construct(\ReflectionClass $context·cfcd208495d565ef66e7dff9f98764da)
            {
                $fields·cfcd208495d565ef66e7dff9f98764da = [];
                $this->a = $fields·cfcd208495d565ef66e7dff9f98764da['a'] = 23;
                $this->b = $fields·cfcd208495d565ef66e7dff9f98764da['b'] = 42;

                parent::__construct($fields·cfcd208495d565ef66e7dff9f98764da, $context·cfcd208495d565ef66e7dff9f98764da);
            }
        }

    class TestClass {}

wtf? o______________0 Its probably #20 issue?

@SerafimArts SerafimArts changed the title Bug or ...? Bug in nested loops or ...? Sep 13, 2016
marcioAlmada added a commit that referenced this issue Sep 13, 2016
marcioAlmada added a commit that referenced this issue Sep 13, 2016
@marcioAlmada
Copy link
Owner

marcioAlmada commented Sep 13, 2016

Hi!
Thanks for trying this so early. A few notes:

1 - Your first case is actually behaving right. You designed a macro that matches a single annotation before a class, so only the green part below is being matched:

- @Any(a = 66)
+ @Some(a = 23, b = 42)
+ class TestClass

2 - There seems to be a huge confusion on how ·ls() works. By default, ·ls means a list of matches separated by a delimiter, and it won't allow a trailing delimiter.:

// so the following parser:
chain(
   token('{'),
   ls(token(T_STRING)->as('letters'), token(',')) ,
   token('}')
)

// will match
{A, B, C} 

// but won't  match
{A, B, C,}
        ^ fails on the last ',' and backtracks to '{'

This means that the parser API needs more care. We certainly need a version of ·ls() that is capable to match a list with a trailing delimiter. Your use case falls on that same culprit as you have:

@Some(foo = bar) T_WHITESPACE
@Any(b = 1) T_WHITESPACE // it's a trailing delimiter! 
class Foo {}

I've just added a ·lst() parser that matches a list and possibly the trailing delimiter, while keeping the old behavior of ·ls.

3 - There are future optimiaztions being planned that could result in whitespace being elided. So in general, ·token(T_WHITESPACE) is not intended to be stable until we have a spec guaranteeing it. A suggestion would be to use ·token(';') as a list delimiter instead, resulting in a more PHP like syntax:

@Any(a = 66);
@Some(a = 23, b = 42);
class MyClass {...}

4 - On your latest attempt, indeed it's a bug. You marked the macro as ·unsafe but the macro hygiene is still renaming the variables. The issue was fixed with commit f6ef3a2.

After fixing the macro hygiene bug, I did this test macro as an example: annotations.

@marcioAlmada marcioAlmada changed the title Bug in nested loops or ...? Bug in nested loops or matching a list with trailing delimiter? Sep 13, 2016
@marcioAlmada
Copy link
Owner

Thanks for reporting this!

@SerafimArts
Copy link
Contributor Author

SerafimArts commented Sep 14, 2016

@marcioAlmada Thank you for such a detailed review!

Lack of complete documentation required study examples (thanks to them) , so I do not quite understand correctly the "entire device" as a whole =) ...not yet.

P.S. More lacks record syntax at the end or beginning of the file, or... inside methods, as example:

@Verify($a > 10);
function a($a) { ... }
// >>
function a($a) {
  assert($a > 10, 'Verify [$a > 10] failed for function ' . __FUNCTION__);
}

I dont know how implement this with yay =(

@marcioAlmada
Copy link
Owner

marcioAlmada commented Sep 14, 2016

I don't know exactly what you're trying to achieve now, it seems t be some form of design by contract using annotations. Something like this would work for the specific case of @Verify:

macro {

    /* DBC macro to check for a condition before a method/function */

    @Verify\Before(···expression);

    ·optional(·repeat(·either(·token(T_PUBLIC), ·token(T_PROTECTED), ·token(T_PRIVATE), ·token(T_STATIC))))·modifiers

    function ·optional(·label())·name (···args) ·optional(·chain(·token(':'), ·ns()·type))·ret_type {
       ···body
    }

} >> {
    ·modifiers function ·name (···args) ·ret_type {
        // @Verify\Before
        assert(··unsafe(···expression), 'Verify "' . ··stringify(···expression) . '" failed for function ' . __FUNCTION__);
        ···body
    }
}

Making a more general purpose DBC framework would be a bit trickier but it's possible. For a more complex example of YAY being used you can refer to yay-enums.

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

No branches or pull requests

2 participants