Skip to content

Conversation

@VictorNogueiraRio
Copy link
Contributor

@VictorNogueiraRio VictorNogueiraRio commented Jun 21, 2021

Missing parts:

1. Tests focused on modules
2. Records as parameters to module functions

"field_names", -- same order as the source type declaration
"field_types", -- map { string => types.T }
},
Module = {},
Copy link
Member

Choose a reason for hiding this comment

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

Can we find a way to not have a module type? I'm happy that we recently got rid of the "fake" types Void and Module and I'm not looking forward to putting them back...

Perhaps by treating the require as special syntax instead of as a regular function call. Or maybe by saying that it returns any.

Copy link
Contributor Author

@VictorNogueiraRio VictorNogueiraRio Jun 21, 2021

Choose a reason for hiding this comment

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

@hugomg using any instead of Module works fine, as well.
Just committed this change now.

Copy link
Member

Choose a reason for hiding this comment

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

Does that mean the user won't be able to shadow require anymore?
perhaps it could be a contextual keyword like async in Javascript.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As of now, the user is still able to shadow require.
I think that's actually a good test case idea.

local imported_func = self.module.imported_functions[upv.f_id]
modules[imported_func.mod] = modules[imported_func.mod] or {}
imported_func.id = upv.f_id
imported_func._tag = "func"
Copy link
Member

Choose a reason for hiding this comment

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

Assigning a _tag is strange. Should we create a proper tagged datatype, using declare_type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough, I think it would be better.

LocalVar = {"id"},
Upvalue = {"id"},
Function = {"id"},
ImportedFunction = {"id"},
Copy link
Member

Choose a reason for hiding this comment

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

Silly question: why do we need separate cases for the Function and ImportedFunction?

Copy link
Contributor Author

@VictorNogueiraRio VictorNogueiraRio Jun 23, 2021

Choose a reason for hiding this comment

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

I tried using Function at first and adding the imported functions to the module.functions table, but there are several places in the code that loop through module.functions and expect there to be a body field. Since we are not running the compiler past the Checker phase ( for obvious reasons), the imported functions won't have a body and, thus, the checks will fail. Also, there are places, such as here (https://github.com/VictorNogueiraRio/pallene/blob/pallene-175/pallene/coder.lua#L1719) where I need to generate a different code if it's an imported function. I mean, I could simply put a field in Ir.Value.Function to differentiate, but, would that really be better? The same goes for ImportedVar.

Copy link
Member

Choose a reason for hiding this comment

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

If the imported functions work differently, I think it is ok to have a separate case for them.

@VictorNogueiraRio VictorNogueiraRio force-pushed the pallene-175 branch 5 times, most recently from 25cdc03 to 2098047 Compare June 23, 2021 19:40
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.

3 participants