Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

add unamed parameter type to references, fixes #423 #462

Merged
merged 3 commits into from
Sep 20, 2020

Conversation

Pangoraw
Copy link
Collaborator

Hi,

This is a fix for issue #423 where methods with unnamed arguments where not added as referencers to the parameters' type. I fixed it by adding a special case in the explore_funcdef function.

This is my first try at touching a serious julia codebase so I will be happy to fix any comments

Thanks 馃巿

@fonsp
Copy link
Owner

fonsp commented Sep 20, 2020

Very nice! Thank you!

@fonsp
Copy link
Owner

fonsp commented Sep 20, 2020

I've added some tricky tests, let's see if they pass!

@Pangoraw
Copy link
Collaborator Author

One is failing, I will take a look

@fonsp
Copy link
Owner

fonsp commented Sep 20, 2020

Thanks! If it's not simple to fix, then we will just merge this PR, and add the failing test as an issue for later.

@Pangoraw
Copy link
Collaborator Author

Alright, it did not seem related. The ... params were send back to explore! (as a default for unknown expressions) were they would be treated as traditional reference to the variable. I added a case to "unwrap" the ... and recurse on it.

Should i make another commit or keep it in the same ?

@fonsp
Copy link
Owner

fonsp commented Sep 20, 2020

Awesome, thanks again! You're right that this was a separate issue, so I'm happy that you fixed this too 馃槉

I added some more tests for the splatting operator (...), they already passed locally so it's probably all good!

@fonsp fonsp merged commit bbd2132 into fonsp:master Sep 20, 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.

None yet

2 participants