free-audio / clap

Audio Plugin API
https://cleveraudio.org/
MIT License
1.77k stars 100 forks source link

fix: make features on plugin_descriptor_t const #196

Closed mnkisala closed 1 year ago

mnkisala commented 1 year ago

I'm not sure if there would be a reason for anything to modify the list of features after instantiating the descriptor (which should take place at compile-time anyway). I was trying to port plugin-template over to Zig and the compiler wouldn't let me cast const [c]const u8 to [c][c]const u8 (obviously that would discard the const, which is allowed in C but not in Zig).

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

baconpaul commented 1 year ago

won't this be an ABI breaking change though?

robbert-vdh commented 1 year ago

It technically would be, although realistically no host would modify this array. So in practical terms this would only break the API and not the ABI.

baconpaul commented 1 year ago

there's not a mangled name change or anything like that? i guess this is c code and not a function or top level global point so the mangler shouldn't be in effect.

robbert-vdh commented 1 year ago

Yes it's just a struct field. The memory layout for a const foo** and a const foo* const* is identical since both are just a pointer. Of course, it's still technically an ABI change because the older version would let you overwrite the feature string pointers while the newer version does not, but in practical terms this won't break anything and this PR's version is more in line with the intended semantics.

robbert-vdh commented 1 year ago

On another note, this PR should be targeting the next branch. Not main.

abique commented 1 year ago

Please include the changelog entry and then it'll be ready to merge I suppose.

abique commented 1 year ago

Thanks!