Skip to content

Added marshalling to struct...#525

Open
genuinelucifer wants to merge 6 commits into
mono:mainfrom
genuinelucifer:structStructMarshalling
Open

Added marshalling to struct...#525
genuinelucifer wants to merge 6 commits into
mono:mainfrom
genuinelucifer:structStructMarshalling

Conversation

@genuinelucifer

Copy link
Copy Markdown
Contributor

Mostly done... But, one other error is exposed due to this. And moreover it now also marshals few C++ classes to C# struct.
Was that intended while using @class.IsPOD @tritao @ddobrev

Also, I did not add the check for is declared as struct this time.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What @tritao had in mind was to use IsPOD instead of IsValueType. So you'd best change it to just:

return @class.IsPOD.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will do.

@ddobrev

ddobrev commented Jul 26, 2015

Copy link
Copy Markdown
Contributor

Marshalling some classes as structs is OK, this is what @tritao 's idea is: marshal types based on their usage, not on the key-word which means almost nothing in C++. The error you get, however, is indeed a problem. What exactly is it?

@genuinelucifer

Copy link
Copy Markdown
Contributor Author

@ddobrev The code marshals a class which was intended to be a Ref type as I had written it for testing a fix when CS_OUT was applied to a ref type argument.
Now it is value type and it is not being properly initialising before use.

@genuinelucifer

Copy link
Copy Markdown
Contributor Author

@ddobrev Also i wanted to ask about the case when a class in completely different assembly inherits from a class which was previously marshalled as Value Type.
I described the problem on #500 a few days back,

@ddobrev ddobrev force-pushed the master branch 4 times, most recently from d45e2da to dd941d9 Compare August 4, 2015 11:20
@genuinelucifer

Copy link
Copy Markdown
Contributor Author

@ddobrev Adding this pass to generator didn't seem to be good. Because of two main reasons:
1. If user has 1 class in one project inheriting from a second class in another project. Then if he uses CppSharp on the two projects separately (As in Namespace Test) then it causes error if the Base has laready been marshalled to value type.
2. Some classes are used in Tests like they are ref type and after marshalling them to value type the tests break.
.
For (1) I have introduced a new macro CS_REF_TYPE which will allow the user to force marshalling to ref type even when CheckClassAsValueTypePass is used just on the condition that the CheckMacroPass is used before this pass.

@genuinelucifer

Copy link
Copy Markdown
Contributor Author

@tritao @ddobrev Please review this.

@genuinelucifer

Copy link
Copy Markdown
Contributor Author

@ddobrev @tritao Any comments?!

@ddobrev

ddobrev commented Sep 4, 2015

Copy link
Copy Markdown
Contributor

Hello @genuinelucifer . Please accept my apologies for this huge delay. I've been really busy with Qt#, I am almost ready to build GUI examples with it and it needs a last effort. Meanwhile please proceed with the task about delegates, it's more important than this one anyway.

@genuinelucifer

Copy link
Copy Markdown
Contributor Author

@ddobrev That's great! :) I had a look into Qt# and it really seems that you have put much much effort into it. Best of Luck!

I'm actually unable to proceed with the delegates task because I actually don't know much about them and actually couldn't figure out how to do that. I will post my real problems on that issue soon.

@ddobrev

ddobrev commented Sep 9, 2015

Copy link
Copy Markdown
Contributor

@genuinelucifer I am glad you have started thinking about the task. Feel free to ask any questions. I still don't have time to review this one but I have more than enough for questions.

@ddobrev ddobrev force-pushed the master branch 2 times, most recently from ad9811b to beabb82 Compare September 10, 2015 22:38
@tritao tritao force-pushed the master branch 2 times, most recently from ff30267 to e6a74df Compare September 27, 2015 21:58
@tritao

tritao commented Oct 2, 2015

Copy link
Copy Markdown
Collaborator

Hey @ddobrev we should review this soon.

@ddobrev ddobrev force-pushed the master branch 2 times, most recently from b7efc4e to c4c2ef2 Compare June 23, 2016 16:18
@ddobrev ddobrev force-pushed the master branch 4 times, most recently from a2a14c5 to f197aad Compare July 12, 2016 20:34
@ddobrev ddobrev force-pushed the master branch 4 times, most recently from 99934be to 6a0069f Compare July 22, 2016 12:00
@ddobrev ddobrev force-pushed the master branch 8 times, most recently from 09fd87d to aabe740 Compare August 7, 2016 16:25
@tritao tritao force-pushed the master branch 2 times, most recently from da1b8ba to 1f25e02 Compare August 10, 2016 16:35
@ddobrev ddobrev force-pushed the master branch 2 times, most recently from 9fc81b8 to cbafb55 Compare August 16, 2016 21:11
@tritao tritao force-pushed the master branch 4 times, most recently from e95c3f8 to 5dac663 Compare August 24, 2016 17:43
@ddobrev ddobrev force-pushed the master branch 4 times, most recently from 4f3ca17 to 6ed05c2 Compare September 13, 2016 21:19
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