Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

ProvideTypePrinters with more context#1788

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
Saalvage wants to merge1 commit intomono:main
base:main
Choose a base branch
Loading
fromSaalvage:feature/type-printer-more-context

Conversation

@Saalvage
Copy link
Contributor

I am absolutely uncertain about this pull request, the underlying mechanisms for this library functionality is still an enigma to me.

What I seek to achieve:
I am still working on the SDL3 bindings. Some of the classes in SDL3 contain fields which are of primitive types (uint32_t) but actually represent enum types. C enums being weakly typed bla bla bla. I would like to map these specific fields (which I hand pick) to the corresponding enum types. For this purpose I thinkTypeMappers are the tool of choice. I have already established similar functionality for function parameters of primitive types that represent enums. I simply collect allParameters and their correspondingType in a dictionary and check that when marshalling.

Trying to replicate this same approach for fields has presented me with the problem that the context that is provided to aTypeMapper is not able to decide whether it's a field that should be mapped. There is very minimal information given, no ability to derive a corresponding class or anything of that sort.

Modifying the library to make what I desire work is fairly trivial. I check for the property inCSharpSignatureType (not the field!) and for the field in the marshalling methods (it's the only thing that's provided respectively).

Alternative approaches I've considered:

  • Changing the field's type inPreProcess and dealing with the marshalling properly via aTypeMapper for the enum
    • Causes a stack overflow for some reason
  • Changing the property's and internal classes field type inPostProcess
    • I can't seem to be able to find the inner__Internal class

Now, the considerations for this pull request:

  • TheParameter field does not actually seem to represent a concrete method parameter, it can be different things for different contexts. Sometimes it's not set at all, other times there's a degenerate version newly created from a differentParameter that has a different naming scheme. So all in all I feel like this PR is not line with the original design intent here (because I currently don't think I understand it).
  • While I only requireField onTypePrinter andProperty onTypePrinterContext I have also decided to include both of them in both classes, as well as an additionalMethod field that could be helpful when trying to recreate the desired functionality for return types.
  • TypePrinterContext'sField property goes unset.
  • Variable is another often-seen class that could be considered for addition. I have decided to exclude it because I cannot see a direct use for it.

I actually hope that what I seek to accomplish can be accomplished using existing library mechanisms, I guess this would've been more fit for an issue asking for help, but I think it might be worth presenting the code here for consideration anyways.

If this PR were to be considered for merging it would definitely require a second closer look by someone with a deeper understanding of the library. It's highly probably that I missed spots for potentially providing the new fields and other times I have provided them needlessly at the very least.

To conclude here's a minimal example that demonstrates what I seek to achieve:

#include<cstdint>typedefenum {    A =0, B =1, C =2} MyEnum;classMyClass {public:uint32_t member_enum;uint32_t member_normal;};

I wantmember_enum to have typeMyEnum on the C# side andmember_normal to remain asuint32_t/uint.

@tritao
Copy link
Collaborator

I think you're finding it a bit awkward to use type maps because fundamentally they are used for mapping types, whereas you want to map a declaration.

I actually hope that what I seek to accomplish can be accomplished using existing library mechanisms, I guess this would've been more fit for an issue asking for help, but I think it might be worth presenting the code here for consideration anyways.

I added the concept of a declaration map before, but it's still very half baked, seems like it's only implemented for the C++ generator. I probably used it for some JS generation stuff I was working on.

publicabstractclassDeclMap

publicclassDeclMapDatabase:IDeclMapDatabase

if(Context.DeclMaps.FindDeclMap(method,outDeclMapdeclMap))

So one possible approach might be extend this. We could even add aDeclaration.DeclMap property, so you could just assign it per decl.

Though I think my idea with theDeclMap feature was more to be able to generate totally custom bindings per decl, whereas what you want is more to slightly hook into the existing generation logic. So not sure if its worth it extending for what you need, maybe since you also need to map how marshaling is done,TypeMap already provides those things, so could be better base to build from.

Anyway take a look and maybe it can give you some ideas.

If this PR were to be considered for merging it would definitely require a second closer look by someone with a deeper understanding of the library. It's highly probably that I missed spots for potentially providing the new fields and other times I have provided them needlessly at the very least.

I think the changes in this PR look fine, development in this library has always been driven by the needs of development of new bindings, so even if this isn't totally complete, we can always improve it in the future.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Saalvage@tritao

[8]ページ先頭

©2009-2025 Movatter.jp