Pass comments to AST nodes during parsing #1005
Conversation
|
Teal Playground URL: https://1005--teal-playground-preview.netlify.app |
| field_comments: {string: {{Comment}}} | ||
| meta_fields: {string: Type} | ||
| meta_field_order: {string} | ||
| meta_field_comments: {string: {{Comment}}} |
There was a problem hiding this comment.
@upedd It's unfortunate that we have the storage of comments mixed across Node and Type, but I understand why you did it this way.
There is a backstory as to why the parser mixes the creation of Node and Type objects — back in the beginning, it felt like a small optimization. But then when I tried to change everything into Node for a better separation of concerns, it became apparent that the checker now had a dependency on this behavior: this is because the type-checker runs as a single pass of the Node tree, and for some situations it becomes suspiciously convenient that the Type objects are already there during the "first pass" (that is, the processing of Type objects as part of the parser functions like a "hidden zero-th pass").
This is eventually fixable, but not something I want us to do right away because changing that has other consequences to the later checking stage (there are other reasons why I'm considering turning the type-checker into a more (nowadays) common multi-pass process).
So we'll keep it the way you did it for now — this was more for sharing and "documenting" the context behind it.
(cc @Frityet since this is also important for you to know)
There was a problem hiding this comment.
For me, this mixing between the Node and Type felt a bit "off". However, it's the best we can do without a major refactor of the parser codebase.
Good to know the context behind it.
hishamhm
left a comment
There was a problem hiding this comment.
This is in the right direction! I think there's one problem spot I caught on review, and the other big thing is a suggestion for a further improvement.
|
@upedd Ah, another thing: I think for consistency's sake we should take any comments that you're attaching to Type objects, and copy them in the |
|
Thank you for your thorough review, @hishamhm. I’ve fixed all the issues you pointed out. Also noticed and fixed some flawed logic in the test code. |
Part of tealdoc project. Passes comments to AST nodes of all declarations, record/interface fields and enum values.