-
Notifications
You must be signed in to change notification settings - Fork 34
Parse pallene types file #657
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
base: master
Are you sure you want to change the base?
Conversation
7868a52 to
c04903a
Compare
| else | ||
| self:unexpected_token_error("a type declaration") | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we encounter an invalid token, the while loop will exit and this function will return as if there was not a problem.
- I think the while should be "while not end-of-file"
- Please create test cases for the "expected a type declaration" errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the while should be "while not end-of-file"
Should we still throw an error if we don't find at least one declaration? If so, we can go with a repeat-until
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should allow empty files, in case the module exports nothing. (Also test that...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to write the error test in a assert_program_error() fashion, but I couldn't get it to work. So I wrote it another way (src/spec/parser_spec.lua:979-990)
src/pallene/typechecker.lua
Outdated
| local type_checker = Typechecker.new() | ||
| if prog_ast._tag == "ast.Program.Program" then | ||
| return type_checker:check_program(prog_ast) | ||
| elseif prog_ast._tag == "ast.TypeFile.Decls" then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This kind of if-else-if doesn't feel right. The type (ast.Program) should be the same in all branches.
I suspect that you actually want to have separate type checking functions instead of a catch-all typechecker.check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same goes to Parser.parse()? Or is it ok for that case?
If I understand it correctly, you are suggesting that we have a new function in the typechecker and the responsibility to choose which one to call must be in driver.compile_internal(). Right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
src/pallene/util.lua
Outdated
| return base_name, parts[#parts] | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it would be simpler to not split the string. We can could the string itself to see if it ends in ".pln" or ".d.pln".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. Also now I see that logic is not necessary, as "." is not an allowed character in file names.
So what I plan to do is this:
- get rid of that new function
util.split_pallene_ext()(andsplit_all_ext) - keep using
util.split_extwith the following modification
function util.split_ext(file_name)
- local name, ext = string.match(file_name, "(.*)%.(.*)")
+ local name, ext = string.match(file_name, "(.-)%.(.*)")
return name, ext
endThat way string.match() will match name only until the first '.', and ext will be the rest of the string. Is that ok?
* Fix logic in `split_ext` and remove unnecessary new functions; * Fix `.d.pln` parsing loop that could stop before file end; * Create a new function for `.d.pln` typechecking. * Add tests
764279d to
e31345a
Compare
hugomg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acho que precisamos de funções separadas para o compile_internal e o parser.parse.
Pense em qual seria o tipo de parser.parse. Hoje é uma função que as vezes retorna um ast.Program e as vezes retorna um ast.TypeFile. Isso até dá pra escrever em Lua, que é uma linguagem dinâmica, mas não funcionaria se quisessemos converter esse código para Pallene no futuro.
Um problema semelhante acontece na compile_internal, que hoje está com um if que verifica a extensão e retorna tipos de dados diferentes dependendo do valor da extensão.
This pull request introduces support for Pallene type declaration files (
.d.pln), allowing the compiler to parse and verify/typecheck type files in addition to regular Pallene source files. The changes include updates to the parser, typechecker, driver, CLI, and utility functions to handle the new file format, as well as new tests to verify correct parsing and type checking of type declaration files.Type Declaration File Support
TypeFileinsrc/pallene/ast.luato represent type declaration files and their components (Typealias,Record,Decl)..d.plnfiles insrc/pallene/parser.lua, including theTypeDeclarationFileparser method and extension-based dispatch in the main parse function. [1] [2]check_type_filemethod insrc/pallene/typechecker.lua. [1] [2]Compiler and CLI Integration (Extra)
--emit-typestosrc/pallene/pallenec.lua, enabling users to generate.d.plnfiles, and integrated it into the main command dispatch. [1] [2]Utilities and Testing
src/pallene/util.luato correctly split and recognize multi-part extensions like.d.pln, supporting robust file name handling.spec/parser_spec.lua.