Default field values & default parameters #810
Conversation
|
Awesome to see this. Very useful features to have in Encore. |
src/types/Typechecker/Typechecker.hs
Outdated
| @@ -1412,7 +1433,7 @@ instance Checkable Expr where | |||
| tcError $ ValFieldAssignmentError name targetType | |||
| | otherwise = return () | |||
|
|
|||
|
|
|||
| -- Karro | |||
There was a problem hiding this comment.
What is Karro?
If this is a comment, it could be removed.
|
There are a number of conflicts with the development branch of Encore. These need to be fixed before we can merge this PR. Do you need help doing that? |
|
hi there! You should do a rebase instead of a merge. If you are unsure about how to do it, let us know. (I am going to check how to fix your conflicts and see if I can push a rebased version. Does this sound good?) |
|
@kikofernandez aha okay it sounds good, I don't know how to do a rebase |
src/parser/Parser/Parser.hs
Outdated
| returnWithEnd Field{fmeta, fmut, fname, ftype} | ||
| optional $ withLinebreaks $ reservedOp "=" | ||
| fexpr <- optional expression | ||
| returnWithEnd Field{fmeta, fmut, fname, ftypr, fexpr} |
There was a problem hiding this comment.
is this returnWithEnd Field{fmeta, fmut, fname, ftypr, fexpr} actually returnWithEnd Field{fmeta, fmut, fname, ftype, fexpr} (misspelling of ftypr to ftype)
|
are you on the slack channel? There are some things that I need to explain and are probably not that relevant for the PR (otherwise send me an email to kiko.fernandez@it.uu.se and we continue some details that should be understood before I do this) |
src/ir/AST/Desugarer.hs
Outdated
|
|
||
| nameForArg :: Int -> String | ||
| nameForArg 0 = "" | ||
| nameForArg x = show x |
There was a problem hiding this comment.
not used. this can be removed
src/ir/AST/Desugarer.hs
Outdated
| @@ -7,23 +7,107 @@ import AST.PrettyPrinter | |||
| import AST.Util | |||
| import Types | |||
| import Text.Megaparsec | |||
|
|
|||
| import Data.Maybe | |||
| import Debug.Trace | |||
There was a problem hiding this comment.
should be removed, this is only for debugging
|
there is a binary file that can be removed too |
knikamo
force-pushed
the
defFieldValues_defParams
branch
from
028de29
to
2cc5f80
Compare
|
@kikofernandez helped us with a rebase and fixing the conflicts. The test for default field values in the main class fails, @EliasC is responsible to fix that. Error messages are not adjusted to the new features. We are going to add issues about this after the PR is merged (right @EliasC ?) |
|
There was a small discussion on cleaning up the commit history a bit before merging. If there's still some pending changes you would like to do, I think it could be nice to add them while revising the history. |
I have this implemented locally now and I believe that it should work out of the box. Should I push the commit directly to this branch?
Yes please! The most important thing is that the known weaknesses are not forgotten. By creating an issue we increase the chances that someone is going to fix it at some point. |
|
@EliasC I think you should push directly to the branch. I do not want to re-write the history until everything that needs to be fixed has been fixed. |
|
Everyone: please let me know when everything is in place (issues have been created, all tests pass, etc) and I will do the interactive rebase to squash some commits together |
|
I've pushed my addition to make the final test case pass. If CI passes, I'm happy with my addition and will not add anything else. @knikamo gets the final word on when everything is in place. |
|
@knikamo: Can Kiko merge your commits and push it here? In other words, are you done and won't be adding anything else here? |
kikofernandez
force-pushed
the
defFieldValues_defParams
branch
2 times, most recently
from
aeee00e
to
803118b
Compare
|
before merging, it would be good if I can get @casperstr and lowe's github email address, so that their commits are linked to their Github users. I'll wait until I get a confirmation on how to proceed |
This commit has squashed the following commits into a single one - removing pbcopy - prepare for default values - exprimenterar med seq väldigt nära nu - default field parameters working with bugs - do not desugar field without expression - refactor. adding comments - deacent looking - adding basic test for default params - moving to other class - More tests - testing expressions and adding out - moving - adding test for defaultParameters - removing binary - adding test for finalizer - adding tests for multiple constructors
- myser lite - funkar lite gran sådär - chaning test - working for functions - testar lite - got first part, (method decl) of default paramaters for methods working. ie this.test1()
This commit has squashed the following commits: - default params for methods fixed - default params for function does not work - default params for functions working - added tests for default params - string as default param in function fixed - removed finalize- and multipleInits-tests - default paramters for local functions fixed - removed a comment - added a test for object as default parameter - edited test for object as default parameter - added test for default field values in a class without an init method - removed comment - git fix - default field values works for class without init - removed files - small fixes
This commit adds an implicit call to `init` at the beginning of the `main` method. It also adds a restriction from defining a constructor method in the `Main` class. "Why this madness?" I hear you ask. This is to give field initialization a place to be desugared into, while still keeping `main` as the starting point of the program.
kikofernandez
force-pushed
the
defFieldValues_defParams
branch
from
803118b
to
7268677
Compare
|
I believe this is ready for merge @EliasC |
|
do not squash |
|
I am an author though, so you should press the button 😃 |
We have implemented two new features, default field values and default parameters during our Independent Project in Information Engineering (similar to a bachelor thesis).
Default field values
Default field values are assignments added at the top of the init-method during desugaring. Default field values does not work for the main class since the init method is not called. Tobias suggested that he would solve this. There is currently no logic to prevent circular references or other unwanted expressions.
Default parameters
For each default parameter in a function/method declaration a new function/method is added during desugaring. During typechecking all function/method calls are adjusted to call the function/method with correct number of arguments. Default parameters work as expected according to the tests we have added.
General
We have not done anything with error handling or error messages. For example the error message for wrong number of arguments is now a bit weird since a function/method not always expects a fixed number of arguments.
We have written a few tests for each feature, but probably they do not cover everything.