Skip to content

Fix for fn call when def argument is interface. #600

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

Merged
merged 1 commit into from
Jan 2, 2016

Conversation

genuinelucifer
Copy link
Contributor

This is my initial thought as I found that desugered is always the class required. I couldn't find a way to figure out if the parameter type is interface. So, probably it will change as currently it generates the cast for every argument whether required or not.

@@ -243,7 +243,7 @@ private static bool CheckFloatSyntax(Type desugared, Statement statement, ref st
if (enumItem != null)
{
result = string.Format("{0}{1}.{2}",
desugared.IsPrimitiveType() ? "(int) " : string.Empty,
desugared.IsPrimitiveType() ? "(int) " : "(" + desugared.CSharpType(new CSharpTypePrinter(Driver)) + ") ",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please break the line after the colon and use string.Format instead.

@ddobrev
Copy link
Contributor

ddobrev commented Dec 30, 2015

@genuinelucifer you should be able to call GetFinalPointee() on param.Type or desugared (not sure which one, you'll have to check) and then use TryGetClass (or a similar name, you'll find it easily, it's widely used). Then you can check Class.IsInterface. Just a note: GetFinalPointee() returns null if the type is not a pointer so you have to use type.GetFinalPointee() ?? type. A really stupid bug we need to fix but for the time being this is the way to ensure you don't get a null crash.

@genuinelucifer
Copy link
Contributor Author

Actually I already tried that and a few other things but couldn't get it to work.
I think that it should be checked while parsing the parameters only. I am trying that now.

@genuinelucifer
Copy link
Contributor Author

@ddobrev I guess it's good to go now. Please review.

@ddobrev
Copy link
Contributor

ddobrev commented Jan 2, 2016

@genuinelucifer Happy New Year! I wish you health, happiness and many more commits to C++#. :)

@genuinelucifer
Copy link
Contributor Author

@ddobrev Thanks and same to you :)

ddobrev added a commit that referenced this pull request Jan 2, 2016
Fix for fn call when def argument is interface.
@ddobrev ddobrev merged commit dd1576b into mono:master Jan 2, 2016
@ddobrev
Copy link
Contributor

ddobrev commented Jan 2, 2016

@genuinelucifer thank you. Let me know when you are ready for more work.

@genuinelucifer
Copy link
Contributor Author

genuinelucifer commented Jan 2, 2016 via email

@ddobrev
Copy link
Contributor

ddobrev commented Jan 2, 2016

Great. :) You've been doing such excellent work that I think you are ready for the next level. There's a relatively simple issue which is very suitable to get you acquainted with the C++ part. #601 requires you to add the new field to our AST.h and then read it in the parser and in ASTConverter. Any questions are welcome.

@genuinelucifer
Copy link
Contributor Author

genuinelucifer commented Jan 2, 2016 via email

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.

2 participants