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

RFC: Remove Delegate #1514

Closed
Theodus opened this issue Jan 11, 2017 · 7 comments
Closed

RFC: Remove Delegate #1514

Theodus opened this issue Jan 11, 2017 · 7 comments
Assignees

Comments

@Theodus
Copy link
Contributor

Theodus commented Jan 11, 2017

Remove the implementation and tests for delegates from the compiler and the explanation and examples from the tutorial.

https://github.com/ponylang/rfcs/blob/master/text/0031-remove-delegate.md

@Theodus
Copy link
Contributor Author

Theodus commented Jan 12, 2017

I have a working implementation of this RFC, but I have a strange problem in the parser macros here. For reference here is the code in the master branch of ponyc.

@Praetonus
Copy link
Member

What kind of problems is it causing?

@Theodus
Copy link
Contributor Author

Theodus commented Jan 20, 2017

When running make test config=debug, if that line is commented out the following error occurs:

libponyc.tests: src/libponyc/ast/ast.c:465: token_id ast_id(ast_t *): Assertion `ast != NULL' failed.
Makefile:648: recipe for target 'test' failed
make: *** [test] Aborted (core dumped)

@Theodus
Copy link
Contributor Author

Theodus commented Jan 20, 2017

Without config=debug, the test for BadPonyTest.EmbedNestedTuple causes a segfault

@Praetonus
Copy link
Member

I think there is a function somewhere that still expects the old AST format for the rule you modified. The fastest way to find it probably is to run the asserting code in a debugger and to print the backtrace.

@Theodus
Copy link
Contributor Author

Theodus commented Jan 20, 2017

I wasn't able to figure out the problem with a debugger. I did find that in flatten.c L227 provides is null and the AST of provider is (embed (id foo) (nominal (id $1) (id Foo) x ref x x) x) when it is expecting (embed (id foo) (nominal (id $1) (id Foo) x ref x x) x x). I'm not entirely sure what the extra x is, but it's the difference between the the tests passing and failing.

@Praetonus
Copy link
Member

An x in the textual representation is a TK_NONE child in the actual AST data structure.

I don't know if you're familiar with the relationship between the parser rules and the resulting ASTs so I'll walk you through it. I'll take the rule causing you trouble as an example.

DEF(field);
  // The first TOKEN of a rule defines the node ID of the AST
  TOKEN(NULL, TK_VAR, TK_LET, TK_EMBED);
  MAP_ID(TK_VAR, TK_FVAR);
  MAP_ID(TK_LET, TK_FLET);
  // Subsequent TOKENs or RULEs create child nodes
  TOKEN("field name", TK_ID);
  // SKIP checks that the expected token is present but doesn't create a child node
  SKIP("mandatory type declaration on field", TK_COLON);
  RULE("field type", type);
  // An IF rule is optional. If the specified token isn't present, a default TK_NONE child node is created
  IF(TK_DELEGATE, RULE("delegated type", provides));
  IF(TK_ASSIGN, RULE("field value", infix));
  REORDER(0, 1, 3, 2);
DONE();

Because an IF rule always creates a child node, the resulting AST will have one child less if you remove one of these rules. The flatten_provides_list will then try to get that nonexistent child and will get NULL back.

With that change, fields won't be able to provide types anymore. To fix the problem in flatten.c, you can simply remove the calls to flatten_provides_list for fields, i.e. here and here.

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

No branches or pull requests

2 participants