[ffi] add HL_ prefix and move outside of hl.h, allow disable legacy definition

[yuxiaomao]

See

• Prefix all defines in hl.h with HL_ #807

As suggested added HL_ prefix and HL_DISABLE_LEGACY_FFI

[tobil4sk]

This looks good! hl_ffi.h also needs to be included in all packaging/install scripts for this to work.

[tobil4sk]

I wonder if it might make sense to do something like this:

hl.h contains legacy ffi definitions only, instead of them being in hl_ffi.h and hl.h including hl_ffi.h. hl_ffi.h only would contain the new definitions, so it is clean. Also hl_ffi.h can define HL_DISABLE_LEGACY_FFI before including hl.h. This way if users migrate to hl_ffi.h they don't have to worry about the bad macro definitions like _GUID, but if they include hl.h directly (but without HL_DISABLE_LEGACY_FFI) they still get legacy definitions as before.

We should also add HL_DISABLE_LEGACY_FFI to hlc templates since ffi definitions are not needed to compile hlc code and it reduces name clashes.

[yuxiaomao]

Seems better indead, will do

[yuxiaomao]

Hum finally I'm not sure: with your idea, when including accidentally <hl.h> before <hl_ffi.h> without HL_DISABLE_LEGACY_FFI, legacy definition will be present; however if the order is <hl_ffi.h> then <hl.h> it does not includes legacy definition. It can be confusing

[tobil4sk]

Hm, you're right that's a good point, it's annoying when header order changes behaviour like that.

I think it would still make sense to keep the legacy definitions out of hl_ffi.h. Also, I think you shouldn't get the new definitions unless you include hl_ffi.h, otherwise the migration is a bit confusing.

If we reorder a few of the definitions we can make hl_ffi.h independent of hl.h. For example, I think HL_NO_OPT isn't really cffi related so it can just go in hl.h, and the vstring declaration as well. This way hl_ffi.h does not have to include hl.h. I think this is better as it makes the purpose of each header more concrete.

[yuxiaomao]

I'm not sure how to deal with the case HL_PRIM/HL_NAME without possible redefinition problem

[tobil4sk]

I'm not sure how to deal with the case HL_PRIM/HL_NAME without possible redefinition problem

HL_NAME should already have #ifndef (or #if !defined()) so that the hdll can define a custom HL_NAME. That way it is safe for it to be in both hl.h and hl_ffi.h.

For HL_PRIM I think a similar solution using #ifndef could be used to avoid redefinition issues.

[yuxiaomao]

Sorry I was not clear: I would like to avoid my use of many ifndef HL_DISABLE_LEGACY_FFI in hl.h around DEFINE_PRIM, and separate the HL_PRIM from DEFINE_PRIM logic. However it seems a little bit complexe due to the !defined(HL_NAME) defines HL_NAME (can't use the same condition in both hl.h and hl_ffi.h)

My current implementation, i.e. use many ifndef HL_DISABLE_LEGACY_FFI, does not introduce HL_PRIM redefinition.

[yuxiaomao]

Do you have other suggestions on this?

[tobil4sk]

Do you have other suggestions on this?

I was hoping it might be possible to just repeat the logic in both hl.h and hl_ffi.h, but seems like HL_NAME makes that difficult, because HL_NAME being unset seems to be used as a way to detect if libhl is being built. Might be good to clean up the logic a bit, that might make this easier.

Ideally hl.h with HL_DISABLE_LEGACY_FFI shouldn't set HL_PRIM, since that is only used for ffi.