vulkan.h collision with future C++ keyword

[krolli]

Hi,
I was just experimenting with C++ modules under Visual Studio in an existing codebase that uses vulkan, and I ran into problem with VkPipelineShaderStageCreateInfo as it has member named 'module'. According to current version of the TS for modules, 'module' is likely to become a keyword in the language and so this produces a bunch of errors.
I'm unsure whether to bring this up with C++ people or here, but since I don't know of any place to report this for C++, I thought I'd start here. C++ folks probably know that adding new keywords will break stuff, so I guess this wouldn't be news for them anyway.

[NicolBolas]

I don't think the C++ committee is going to choose or not choose to keyword module based solely on Vulkan using it as a member name. They're aware that keywords can break code, and they're aware that "module" is an identifier that has significant use in the world.

The question is really how can Vulkan get out of C++'s way on this. Would it be possible to create a VkPipelineShaderStageCreateInfo2, but one which has the same layout (and stype value) as the original?

[ratchetfreak]

the names of the fields in pod structs don't really matter, only the order and sizes. So a rename would be the simple option

[ghost]

We noted this discrepancy last week in fact, and are looking into it. I'm not sure what the agreed upon resolution was because for some reason it didn't get written into the internal bug, but I know there is a resolution here which wasn't just "tell C++ to go away".

Note that @ratchetfreak is correct though - the binary interface is preserved if we renamed the struct member, so existing projects using the current header would continue to work. Only developers updating the header would have any issues, and the expectation is that this member is likely not referenced in too many places throughout anyone's codebase (so impact should be small).

I'll try to keep you posted when we've agreed a resolution on this one.

[fraggamuffin]

We have submitted this paper to the C++ Std as to object to the naming, as this is really the last chance when they are triaging Modules Ballot comment from the National Body and each nation/company has a lot of power to change it at this moment. Please consider asking your company rep to support this paper at the next C++ Std meeting in Albuquerque on Nov 6-11.
P0795R0_ From Vulkan with love_ a plea to reconsider the Module Keyword to be contextual.pdf

[krOoze]

@fraggamuffin With due respect, there is some hierarchy of things. Language takes priority over API.
I would rather have a sensible keyword in C++ and sacrifice naming in Vulkan, than other way around.

[ghost]

@krOoze I don't think anyone is proposing to not add the feature here? In fact, anyone saying that is mad IMO - it's LONG overdue.

The issue is simply the way the keyword is being added as fully reserved rather than contextual - which is the subject of the proposal - not outright removal. As pointed out in the proposal, lots of other projects use the word module as well, we're not the only ones. Making the keyword contextual would basically let us have our cake and eat it, though I do wonder if there are any drawbacks...

[krOoze]

@TobiasHector I see. My concern was so that the c++ does make the keyword something obscure or long in C++20.

lots of other projects use the word module as well

It means it is a good, descriptive keyword. Otherwise nobody would use it in the first place.

Making the keyword contextual would basically let us have our cake and eat it, though I do wonder if there are any drawbacks...

Probably more complicated tokenizer. There is a reason C++ have reserved keywords in the first place.

[NicolBolas]

Making the keyword contextual would basically let us have our cake and eat it, though I do wonder if there are any drawbacks...

The main drawback is that it may not be reasonably possible.

C++'s current contextual keywords, final and override, work contextually because they appear in places in the grammar where identifiers cannot appear. If you see class Identifier, the only things that can appear after that are : (for base classes), ; (for class declarations), and { (for starting the class definition). Therefore, the grammar can allow you to put the special identifier final there and make it contextually work like a keyword. Something similar happens for override at the end of member function prototypes; only keywords and symbols can appear there, so adding a single identifier in that location works with the grammar.

Making module contextual is much harder, grammatically speaking. Identifier1 identifier2; already has grammatical meaning in C++. So giving it a new meaning if Identifier1 just so happens to be "module" (but only if it appears in a certain place) is a rather new thing for C++. Especially since this new meaning will only be new when it happens to be at global scope.

It's not impossible for compilers to do this, of course. But considering that the "soft keyword" idea was ultimately rejected by the committee, odds are that the committee would either keep the module keyword as is or change it to something else, rather than going the contextual route.

[ghost]

Frankly I don't think there's a particularly good option here, just a selection of vaguely bad ones. I'll let the C++ committee figure out what they think is best, but no harm in presenting proposals to them. If they decide "actually yes this is fine to make contextual", then there's no issue. If they don't make that decision, then we (and other projects) will rename a bunch of things, breaking source compatibility, but maintaining binary compatibility (which probably isn't the hugest deal).

[ghost]

It'll be fun if it turns out any of those projects use "module" as a function or class name though 🤔

[fraggamuffin]

Thanks for the great discussion here. Its very healthy to see these different points of view even within Vulkan. I learned a few things I didn't know or had forgotten. The same is also true within the C++ Std. In this case and in many other C++ design decisions, the decision is tough no matter which way you cut it (paraphrasing Tobias) and thats why the experts don't all agree (and why we are not paid minimum wage:) . At the end, C++ will have to weigh how much changes will be required of the outside and of C++ implementations. They may have already considered this, but sometimes presenting new data can sway the hard borderline decisions. I and others have done this in the past, and from a timing perspective, this is the only real window (when ballot is being returned and comments to the TS is reviewed) so why not try it.
This is why in our proposal title, we asked politely for them to 'reconsider', rather then making a demand 'shall be ...' and gave it a much softer and a lighter/funnier touch. I find politeness here goes much further to get a fair hearing then demands in my 15 years experience in C++ as there are a lot of people a lot smarter then me so I respect their thoughtful opinion as I respect this groups' choices. In the end, even I may change my own mind on this as I hear new evidence.

[krOoze]

@fraggamuffin @TobiasHector Alright. But if they standardize something like __module or imodulizarivinator then I blame y'all :P

[ghost]

I will gladly take the fall if they actually call it imodulizarivinator 😆

[krOoze]

@TobiasHector PS: C++ is probably a "binding" in respect to Vulkan. So I think you would be within your rights to change the parameter name if C++20 is detected (though it would still be a problem for experimental module support, which already uses the keyword, and I don't think there is a detection macro).

PS: Alternatively make the user choose through e.g. VK_CPP20_COMPAT macro.

[krolli]

Well, it's interesting to see that what started as "not sure if anyone noticed so let's mention it somewhere" started so much discussion. From the start I was wondering why Modules TS took the path of normal instead of contextual keyword. I get that those are harder to define (for reasons mentioned above), but it seems like something that would break less existing code, which, to me, would have been worth the effort.

Anyway, I just noticed that this has been discussed at the WG21 meeting this week, so I was wondering what is the current status.

@TobiasHector If imodulizarivinator makes it, I will consider it my lifetime achievement. :D

@krOoze I have to say, the macro approach seems like the most reasonable one to me (assuming module remains non-contextual keyword). Given how there already are things like VK_NO_PROTOTYPES and VK_DEFINE_NON_DISPATCHABLE_HANDLE it probably wouldn't feel so out of place either. Enabling it by default only when compiling in C++20 and eventually phasing old name out would probably give enough time for people to transition.

[tomaka]

@fraggamuffin With due respect, there is some hierarchy of things. Language takes priority over API.
I would rather have a sensible keyword in C++ and sacrifice naming in Vulkan, than other way around.

However please take into account the fact that Vulkan is supposed to be just an ABI, ie. a set of functions with precise names, and that vulkan.h exists only for convenience.

Therefore deprecating functions and adding new ones because of this keyword problem is totally wrong from a purely theoretical point of view.

[krolli]

However please take into account the fact that Vulkan is supposed to be just an ABI, ie. a set of functions with precise names, and that vulkan.h exists only for convenience.

Therefore deprecating functions and adding new ones because of this keyword problem is totally wrong from a purely theoretical point of view.

I think we can all be happy that ABI compatibility is not affected by this and, given that C++ uses snake_case while Vulkan went with vkCamelCase, it should be safe for some time to come. Although, I am a bit worried about proposals to deprecate POD ...

[NicolBolas]

@krolli

Although, I am a bit worried about proposals to deprecate POD ...

Don't be. They're deprecating the term Plain Old Data, and it's being deprecated due to lack of use. From National Body comment US 101:

The term POD no longer serves a purpose in the standard, it is merely defined, and restrictions apply for when a few other types preserve this vestigial property. The is_pod trait should be deprecated, moving the definition of a POD type alongside the trait in Annex D, and any remaining wording referring to POD should be struck, or revised to clearly state intent (usually triviality) without mentioning POD.

The reason this is the case has to do with changes made in C++11. During that process, the committee looked at the concept of POD and realized that it was very much overspecified. It conflated two distinct concepts that didn't really need to be joined.

So C++11 split POD into two constructs that are fundamentally unrelated to each other: the ability to memcpy an object (trivial copyability and trivial types) and an object whose member layout was essentially what you see (standard layout). A POD type was redefined to be a type that is both trivial and standard layout.

But these are essentially orthogonal. The layout of a class doesn't change just because you have a user-defined copy constructor. So the guarantees that standard layout can provide will still be guaranteed in such cases. Likewise, the ability to memcpy a type doesn't go away just because you have data members in both the class and one of its base classes. So the guarantees of trivial copyability can still be preserved in such cases, even though layout compatibility cannot.

As such, the standard since C++11 almost never refers to types that have the combination of these two features. Just do a search in the standard for "POD"; it's almost never referenced. C++ is more likely to talk about the components of POD (triviality and standard layout) than their intersection. Even C features like offsetof are based on the type being standard layout, not POD (that is, the standard requires that implementations not disturb the layout of an object when you add member functions).

So it's not that C++ is losing any particular feature. It's just a wording change; they found a better way to describe the concept, which more types can fit into. And there's no reason to specify the behavior for the intersection of these features, since they're orthogonal to each other.

[krolli]

@NicolBolas Thanks for clarification. I thought that was the case and you've confirmed it for me. I just don't trust my understanding of standardese enough to feel confident. :)

[krOoze]

@krolli Header config macros are evil. But as you say, considering there already are some... (though in #313 I was proposing a way to get rid of VK_NO_PROTOTYPES)
Similarly here it would potentially be better to have separate header.

For fun, let's have us a PR to that effect: #616

I notice there is proposal already in C++: P0795R0

[krolli]

@krOoze Yeah, it's the proposal that @fraggamuffin mentioned above. :)
Looking at the stuff you linked, it ocurred to me, that one more option would be to treat module-based C++ as a separate thing from header-based C++. That is, having a different generator that would spit out module interface file (.mpp, .ixx, whatever). Off the top of my head, I can think of a few things worth noting:

1. It would contain Windows.h mess inside the module interface, which is what they were designed for in the first place.
2. It provides a relatively clean way of dealing with keyword collision. I agree with opinion that it's best if Vulkan avoids these collisions where possible, but it might not be an approach that scales well with number of language bindings over time. New keyword in single language requiring changes in existing code in all languages might not be acceptable, though how often this would happen in reality is another matter.
3. It would cause problems when switching modules on, as #include <vulkan.h> would have to be replaced with import vulkan;.
4. There may be some practical issues with this that would show up when used in real world scenario. What would happen if multiple projects (ideally some being 3rd party) referenced this module? Who is responsible for building it? A lot of things that will likely come down to how build systems integrate this.
5. And then there is the issue of header config macros which you can't really use with modules.

I'm honestly not sure this is the way to go in this case. There are too many things about modules that are not clear to me yet, most having to do with build systems rather than C++ syntax.

[NicolBolas]

You cannot reasonably treat modules as though it were some kind of dialect of C++. It will be a long time before applications can reasonably be "all modules" or "all not modules". During that period, some libraries will use modules in some places. Some applications will use modules in some places. This will be a transition, not some kind of clean break.

And therefore, you still need to be able to generate a vulkan.h that doesn't break C++.

Normally, my feeling is that language binding generators need to take into account the possibility that Vulkan will create identifiers that conflict with language keywords. That it is not the Khronos group's responsibility to check every identifier with every language's keywords. But C++ is a special case because, unlike most languages that can interface with C, it interfaces with C directly. It can consume (carefully coded) C headers as if they were C++ headers.

And therefore, C++ users of Vulkan have every right to expect to be able to consume the C headers for Vulkan.

[krOoze]

Well, it consumes C++ headers. C only happens to be mostly C++ subset.
There's no fundamental reason why C and C++ users have to have the same header file. Except it is practical — that does not IMO apply here though, because the headers are autogenerated.
Unfortunately the name vulkan.hpp seems to be already taken by a project that has higher ambitions than mere C++ compatibility.

Anyway,
The styleguide is getting bit unwieldy. But perhaps there should be a note that despite Vulkan being C99 authors should try to avoid any newer and upcoming known C as well as C++ keywords...
Luckily it is also something that can be automatically checked. That is if someone has the time to invest in improving scripts in this repo...

[ghost]

So @fraggamuffin is rather busy so hasn't had a chance to update this bug, but I managed to grab him for a bit and he confirmed that the C++ committee agreed to make "module" into a contextual keyword, for apparently a number of reasons, and there was some commitment to make this change to any implementations quickly.

In light of that decision, we believe there is no reason to make any change to the Vulkan specification or headers. The only remaining issue is for anyone using the experimental features in VS 2017, but as these are experimental features and not intended to be bug free, we don't plan to make any changes in order to make those work. If anyone needs it to work, a simple rename from "module" to any other sane unique identifier in the header will function correctly, but we don't intend to pull that into the core header.

I will close this issue, and will similarly close #616, based on that resolution.

@krolli if you have any remaining issues, please feel free to comment again here (re-opening if appropriate) or raise a new issue.

[cirk2]

Hello,
While the proposal by @fraggamuffin (p0795r0) has met consensus in Albuquerque the concrete follow up poposal (P0924r0) has been voted against. So in it's current form the Module TS and C++20 is still reserving 'module' as non-contextual keyword. Which invalidates the reason to close this ticket.
Therefore I propose for this ticket to be re-opened.

[fraggamuffin]

Yes, I have confirmed they have reversed their decision. I am trying to chase it down as to the reason why. More later.

…

On Tue, Jun 5, 2018 at 7:58 AM, Felix Hellmann ***@***.***> wrote: Hello, While the proposal by @fraggamuffin <https://github.com/fraggamuffin> (p0795r0) has met consensus in Albuquerque the concrete follow up poposal (P0924r0) has been voted against. So in it's current form the Module TS and C++20 is still reserving 'module' as non-contextual keyword. Which invalidates the reason to close this ticket. Therefore I propose for this ticket to be re-opened. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#568 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGGQSc2owHJmmTbrGCqSTq18_AlAye6Aks5t5nJ4gaJpZM4Ph14R> .

[Tobski]

Re-opening issue so that it stays on our radar and doesn't just catch us off guard in however many months/years.

[skilz80]

Instead of breaking code bases that already exist yet while preserving the use of this as a key word. The C++ language forbids the use of identifiers that begin with an _ as that is reserved for the language and for compilers. So wouldn't it make sense if they changed the key word module to something like _module, __module, or _module_, then user's of the language can still continue to have their identifiers as module without breaking anything, without having to go and replace their identifiers and ship out replacement code for everyone who uses their product. This is just my opinion of the simplest solution. So I tend to think that this should be placed in the hands of the Standard Committee to enforce different compilers to implement module as they see fit with a version of the _ prefix. So for example Visual Studio would have it as __module__ where gcc could have it as _module and it would still work as intended, then Khronos Group - LunarG wouldn't have to change their source and break the thousands or millions of applications that happen to use it.

[krOoze]

@skilz80 Why though butcher our beloved language over something that can be refactored in the codebase in 10 seconds?

PS: fwiw in current working draft module is not a keyword, but an "identifier with special meaning".

[skilz80]

@krOoze Oh okay it looked or appeared to be a key word, didn't realize it was an identifier with special meaning, I might of thought that at first because of Visual Studios context highlighting. But it was just a thought though. There is nothing wrong with the language, but if was prefixed with a single _ since all names that start with _ are reserved for both the language and for different compilers that would leave the identifier module available for anyone to use it for naming any of their components within their code base without any conflicts. That is all I was saying. I know that Khrono could easily change their member's name, but there may be 100's or even 1000's of other libraries out there that might of done the same so that would mean if it conflicts with existing code; that would require 100,000's to 1,000,000's of pieces of software having to be update. But then the argument can go the other way too. If the special identifier were to be changed via an update to one's compiler then any code that was using that compiler flag would then have to be updated too... So in the end I think it would left up to first the C++ standard, then to the Compilers then finally to the Libraries - APIs...

[NicolBolas]

@skilz80 Point of order:

as that is reserved for the language and for compilers.

In C++, names prefixed with two underscores, or an underscore followed by a capital letter, are reserved for implementations for any use (macros, keywords, etc). Names prefixed with a single underscore are reserved only for global namespace identifiers. As such, users are allowed to use them for anything that wouldn't conflict with a global name (an in-struct identifier, a function parameter, local variable, etc).

In any case, while the ISO C committee is willing to prefix names with __ or underscore+capital to avoid conflicts (with a macro imported from a header typically used to make the keyword legible), the ISO C++ committee is far less willing to force users to use ugly keyword names just to avoid conflicts. The thinking is that, if it's important enough to keyword, then it's important enough to use a keyword that doesn't poke you in the eye when you read it.

[hannes-harnisch]

This can be closed. module is a contextual keyword in C++20. There are no conflicts. Not will be, there are none. I can attest that structs using module as a member name of any kind or other kinds of identifiers using module are unproblematic in C++20. Everything compiles just fine.

The only restrictions on C++ identifiers using the word module are that C++20 modules themselves and module partitions cannot be named module, which are constructs only existing in C++20 anyway.

[oddhack]

Closing - thanks for the update @hannes-harnisch !