PMunch / futhark

Automatic wrapping of C headers in Nim
MIT License
394 stars 22 forks source link

incorrect "anon" struct fields added #90

Closed n0bra1n3r closed 9 months ago

n0bra1n3r commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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

n0bra1n3r commented 9 months ago

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