-
Notifications
You must be signed in to change notification settings - Fork 190
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
ash: Hide provisional extensions behind new provisional
feature flag
#952
base: master
Are you sure you want to change the base?
Conversation
BTW, I don't know that it impacts anything you're doing here, but just to note that in the C bindings, the provisional types and commands are kept in a separate 'beta' header, but the provisional enums added to existing enumerated types have to be visible in the core API header as there is no way to define them separately from the type they are part of. |
They could remain in the same file - it's only a handful - and individual enum variants or constants can also be hidden behind a |
Provisional extensions are susceptible to **breaking** API changes at any point (with accompanying `SPEC_VERSION` bump). By hiding them behind a non-default feature flag, we can document that opting into this feature allows us to do breaking changes to these modules in non-breaking `ash` releases when driven by upstream `Vulkan-Headers` changes to `vk.xml`. Moreover, upstream Khronos repositories that build `ash` in their CI can continue to build-test their changes without compile-testing any extension behind a `provisional` flag, which is especially useful if we've already wrapped a `provisonal` extension that is seeing breaking changes.
404e7dc
to
6bc7a61
Compare
Working on this PR is really showing that the generators' lack of building definitions etc. top-down while being aware of their enabling feature(s)/extension(s) is such a hassle to build back in. It takes quite a while to push that information back in and is messy while at it. |
6bc7a61
to
020d26d
Compare
020d26d
to
6b92340
Compare
Fixes #343
Provisional extensions are susceptible to breaking API changes at any point (with accompanying
SPEC_VERSION
bump). By hiding them behind a non-default feature flag, we can document that opting into this feature allows us to do breaking changes to these modules in non-breakingash
releases when driven by upstreamVulkan-Headers
changes tovk.xml
.Moreover, upstream Khronos repositories that build
ash
in their CI can continue to build-test their changes without compile-testing any extension behind aprovisional
flag, which is especially useful if we've already wrapped aprovisonal
extension that is seeing breaking changes.One important feature that is missing in this PR and the reason for its draft state is that we don't yet wrap any types
<require>
d by provisional changes. While these types are unlikely to be useful without enabling the extension, non-breaking releases with breaking changes to such types should be considered invalid. And there may be provisional extensions that solely consist of new types (extending existing types viapNext
).Secondly, feature flags may not technically exempt us from semver compatibility in releases, even if the feature flag is documented as such. We could "do this anyway" for simplicity, or move to something more aggressive like
cfg
definitions.