Skip to content

feat: function calls#9

Merged
Wodann merged 17 commits into
mun-lang:feature/dispatch-tablefrom
baszalmstra:function_calls
Oct 19, 2019
Merged

feat: function calls#9
Wodann merged 17 commits into
mun-lang:feature/dispatch-tablefrom
baszalmstra:function_calls

Conversation

@baszalmstra
Copy link
Copy Markdown
Collaborator

@baszalmstra baszalmstra commented Oct 5, 2019

This PR adds the ability to use function calls in Mun.

  • Type inferencing for function calls
  • Diagnostics on invalid parameter count and types
  • Code generation to be able to call the functions
  • IR generation tests
  • Dispatch table generation
  • IR call through dispatch table
  • Integrate runtime and compiler in a test suite
  • Optimize the shared object

@baszalmstra baszalmstra self-assigned this Oct 5, 2019
@baszalmstra
Copy link
Copy Markdown
Collaborator Author

Function calls

@baszalmstra baszalmstra changed the title feat: function call inferencing feat: function calls Oct 12, 2019
@baszalmstra baszalmstra marked this pull request as ready for review October 15, 2019 07:50
@baszalmstra baszalmstra requested a review from Wodann October 15, 2019 07:50
@baszalmstra baszalmstra changed the base branch from master to feature/dispatch-table October 15, 2019 08:03
Copy link
Copy Markdown
Collaborator

@Wodann Wodann left a comment

Choose a reason for hiding this comment

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

Some general feedback:

  • You use a lot of comments to describe what a section of code or line of code is doing. I am a big proponent of writing self-explanatory code. By moving that code to separate functions whose names describe what happens, you can write a much more concise function body using those function calls.
  • Merge commits can break tools such as git blame, so instead try to use git rebase
  • Try to squash fix commits into their respective feature commits to clean up your history in a feature branch
  • Try to create separate PRs where possible and order them such that they can be applied consecutively. E.g. first merge a PR for temporary directories. Then merge the PR for function calls that uses the previous PR. It ends up being easier to review smaller PRs with related materials.

Comment thread crates/mun/test/main.mun Outdated
Comment thread crates/mun/test/main.mun Outdated
Comment thread crates/mun_codegen/src/code_gen/symbols.rs Outdated
Comment thread crates/mun_codegen/src/ir/dispatch_table.rs Outdated
Comment thread crates/mun_codegen/src/ir/dispatch_table.rs Outdated
Comment thread crates/mun_codegen/src/ir/function.rs Outdated
Comment thread crates/mun_codegen/src/ir/function.rs Outdated
Comment thread crates/mun_codegen/src/ir/function.rs Outdated
Comment thread crates/mun_codegen/src/ir/function.rs Outdated
Comment thread crates/mun_codegen/src/lib.rs Outdated
@baszalmstra baszalmstra force-pushed the function_calls branch 4 times, most recently from 4beef16 to c2b285f Compare October 15, 2019 22:37
@baszalmstra baszalmstra requested a review from Wodann October 15, 2019 22:37
Co-Authored-By: Wodann <Wodann@users.noreply.github.com>
@baszalmstra baszalmstra requested a review from tdejager October 16, 2019 10:27
Copy link
Copy Markdown
Collaborator

@tdejager tdejager left a comment

Choose a reason for hiding this comment

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

Very good, well done. There are a lot of files though so it was hard to do a thorough review, might be better to split some more stuff later on. But as always I'm impressed.

Comment thread crates/mun_codegen/src/code_gen.rs
Comment thread crates/mun_codegen/src/code_gen/symbols.rs
Comment thread crates/mun_codegen/src/db.rs
Comment thread crates/mun_codegen/src/ir/dispatch_table.rs
Comment thread crates/mun_codegen/src/ir/function.rs
Comment thread crates/mun_runtime/src/assembly.rs
@baszalmstra baszalmstra requested a review from tdejager October 17, 2019 12:34
Comment thread crates/mun_codegen/src/ir/dispatch_table.rs Outdated
Comment thread crates/mun_codegen/src/ir/dispatch_table.rs Outdated
Co-Authored-By: Wodann <Wodann@users.noreply.github.com>
@Wodann Wodann merged commit d42805c into mun-lang:feature/dispatch-table Oct 19, 2019
@Wodann Wodann added this to the Mun v0.1.0 milestone May 14, 2020
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