Please bump GLSLANG_PATCH_LEVEL after major API changes

[haasn]

In git master you removed the NV_EXTENSIONS macro checks from the TBuiltInResources, which every user is forced to copy/paste from the glslang examples. Since having done this, you didn't bump the GLSLANG_PATCH_LEVEL, so you broke the compilation of every single dependent without a way to conditionally compile. So as it stands, users can either write code that works only on master but not on the latest release, or write code that works only on the latest release but not master.

Can you please update the GLSLANG_PATCH_LEVEL, ideally every time you break the core API?

(As an aside, I don't understand why glslang doesn't just provide its own TBuiltInResources in the headers, so that users can just use it instead of copy/pasting it from the examples. About 50% of my glslang code is just this struct definition. You already define it in StandAlone/ResourceLimits.h but this header does not get installed as part of the public API - which is odd because the CMakeLists references a library glslang-default-resource-limits that doesn't seem to actually get built. What's up with this?)

[CounterPillow]

imho they are already doing so much for the community with quality tagged releases such as "master-tot", "untagged-048c4dbc7f021224a933", and "Overload400-PrecQual". Do you know how much work it is for them to increase GLSLANG_PATCH_LEVEL? They need at least 30 minutes to launch Visual Studio.

[johnkslang]

We are indeed using stable tags. Before making the last set of large changes, the stable release tag 7.9.2888 stable release was made. This would be the one to continue using until the next one is made.

It is likely time to make a new stable tag, based on the settling down of that recent large set of changes, unless we need to deal differently with these changes.

I have been updating the patch level, etc., on functional changes, but it is worth considering for this kind of change as well. The patch level could be changing basically every commit, but I'm not sure that is necessary. Perhaps you wanted the minor version bumped (the middle number).

About the actual problem; I'm not sure I understand why this is a problem. It was strongly requested by multiple IHVs to not have #ifdef in the interface. The guidance was this should not break things, as it should be okay to have extra ignored fields in an interface data structure without breaking consumers setting/reading of a subset of them.

The glslang-default-resource-limits should be building ResourceLimits.cpp to set up the resources for glslang.

If there is something to fix about the IHV #ifdefs, I'd like to fix that, and then make a new stable release soon.

[haasn]

The guidance was this should not break things, as it should be okay to have extra ignored fields in an interface data structure without breaking consumers setting/reading of a subset of them.

There are a number of issues with this approach:

1. You added the new fields before the TLimits limits, so the normal C++ way of initializing structs (enumerating all members in order) breaks - what was previously the definition for limits is now the definition for maxMeshOutputVerticesNV
2. The normal C way of avoiding this issue (designated initializers) is not allowed in C++ for whatever reason
3. Since the struct is public, sizeof(TBuiltInResource) is a part of the ABI. Changing this size constitutes an ABI break. (Not as big an issue because glslang builds static libraries by default, but it's still worth pointing out in case you ever decide to change that)

I think it falls down to the exact interpretation of "extra fields". This issue could most likely have been avoided by moving the new fields to the end of the struct, after the TLimits.

The patch level could be changing basically every commit, but I'm not sure that is necessary.

Judging by how high (numerically) the patch level is already, I'd gathered that was the intent? I mean it boils down to a question of what the goal here is:

1. If the patch level exists to allow marking bug fix releases as such, then there's no reason the patch level needs to be that high, and also no reason to increase it on API changes
2. If the patch level exists to signal API changes on master, then it needs to be bumped every time you change the API in any way (whether that's due to a new addition, a removal, a change in semantics or modifications to public structs and classes).

Given that the current patch level leans more towards 2 than towards 1, I'd suggest going all the way with it and changing it whenever needed - even if that's basically every commit. (It's also IMO unnecessary to have it be part of the release version numbers, instead you could just enumerate those 7.9.0 7.9.1 etc. as needed. If the primary use of the patch level is for conditional compilation, then it already serves that purpose even without being part of the version number)

If the intent is for users to avoid git master and instead always use tagged releases, then it makes no sense for the patch level to increase at all except on new tags.

It was strongly requested by multiple IHVs to not have #ifdef in the interface.

I'd also tend to agree with this, FWIW.

My issue is not the removal of the #ifdef, but the fact that there's no way to write code that's backwards compatible with older versions of glslang (especially the latest release version). See e.g. gentoo/gentoo#10101 for an example of a project that is bitten by this.

[johnkslang]

So, the incremental action here is to more consistently bump the minor version for changes of this size (rather than just the patch version).