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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incorrect "anon" struct fields added #90

Closed
n0bra1n3r opened this issue Jan 30, 2024 · 6 comments
Closed

incorrect "anon" struct fields added #90

n0bra1n3r opened this issue Jan 30, 2024 · 6 comments

Comments

@n0bra1n3r
Copy link

Thanks for this package. Just wanted to report an issue I found with generated POD structs. Given the C struct:

struct EngineCreateInfo
{
    Int32                    EngineAPIVersion;
    Uint32                   AdapterId;
    Version                  GraphicsAPIVersion;
    const ImmediateContextCreateInfo* pImmediateContextInfo;
    Uint32                   NumImmediateContexts;
    Uint32                   NumDeferredContexts;
    DeviceFeatures Features;
    Bool                EnableValidation;
    VALIDATION_FLAGS    ValidationFlags;
    struct IMemoryAllocator* pRawMemAllocator;
};

Futhark generates the following nim types:

type
  structimemoryallocator* = distinct object
  structenginecreateinfo_anon0_t* {.pure, inheritable, bycopy.} = object
  structenginecreateinfo* {.pure, inheritable, bycopy.} = object
    Engineapiversion*: Int32
    Adapterid*: Uint32
    Graphicsapiversion*: Version
    pimmediatecontextinfo*: ptr Immediatecontextcreateinfo
    Numimmediatecontexts*: Uint32
    Numdeferredcontexts*: Uint32
    Features*: Devicefeatures
    Enablevalidation*: Bool
    Validationflags*: Validationflags
    anon0*: structenginecreateinfo_anon0_t
    prawmemallocator*: ptr structimemoryallocator
  Enginecreateinfo* = structenginecreateinfo

This results in erroneous offsets when the generated struct is passed back into C.

@n0bra1n3r
Copy link
Author

There is a similar issue with the following construct:

struct SomeStruct {
  union {
    int a;
    float b;
  };
  string c;
};

Futhark seems to not recognise the union and merges it's definition with other fields. I'll look for a concrete example of this if you need to reproduce.

@PMunch
Copy link
Owner

PMunch commented Jan 30, 2024

I actually just noticed the second behavior you mentioned yesterday and fixed it in my local repository. Plan was to tidy it up and push it today! Not quite sure what goes on in the first case, it seems to just randomly add a field. I'll have to look into that one.

@PMunch
Copy link
Owner

PMunch commented Jan 30, 2024

Pushed my fix now. Could you update to 0.12.1 and check if it also somehow solves your first issue? If not it would be great if you could create a minimal sample that reproduces the behaviour.

@n0bra1n3r
Copy link
Author

n0bra1n3r commented Jan 30, 2024

Thanks for looking into this. I've come up with a minimal example for both cases, using version 0.12.1:

// futhark.h

struct struct_1 {
  union {
    int int_field_1;
    float float_field_1;
  };
  int int_field_2;
};

struct struct_3 {
  struct struct_2* struct_field_1; // struct_2 is forward-declared
};
# futhark.nim

import pkg/futhark

importc:
  path "./"
  outputpath "futhark_gen.nim"
  "futhark.h"

I generated futhark_gen.nim with nim c --compileOnly --define:nodeclguards futhark.nim:

# futhark_gen.nim


type
  structstruct2* = distinct object
type
  structstruct1_anon0_t* {.union, bycopy.} = object
    intfield1*: cint
    floatfield1*: cfloat
  structstruct1* {.pure, inheritable, bycopy.} = object
    anon0*: structstruct1_anon0_t ## Generated based on /Users/ryan/src/tests/futhark.h:1:8
    intfield2*: cint
  structstruct3_anon0_t* {.pure, inheritable, bycopy.} = object
  structstruct3* {.pure, inheritable, bycopy.} = object
    anon0*: structstruct3_anon0_t ## Generated based on /Users/ryan/src/tests/futhark.h:9:8
    structfield1*: ptr structstruct2

The union issue is fixed (excellent!!), but the forward declared pointer issue is still there.

@PMunch
Copy link
Owner

PMunch commented Jan 31, 2024

This bug should be fixed now, please try it out and verify that it works with your original library.

@n0bra1n3r
Copy link
Author

Thank you, I've tested on the library, and it's fixed!

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

No branches or pull requests

2 participants