-
-
Notifications
You must be signed in to change notification settings - Fork 618
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
Implemented VS API for toggling of C++20 modules and STL module build #2130
Conversation
|
||
function m.buildStlModules(cfg) | ||
if _ACTION >= "vs2022" then | ||
if cfg.buildstlmodules then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to have this one activated, and not the other? (to emit warning for strange config).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, it makes no sense to have this activated wtihout the other, but I don't believe we do this kind of sanity checking elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
4c6588e
to
912aeae
Compare
I appreciate you adding this, makes it easier for a lot of us to experiment and or use it. |
Does |
Bumping this @vulcan-dev |
I apologise, I must have had to go when I was responding last time. It does not, I remember having issues regarding modules, and I had to enable the other option. In my temporary fix, I have it enabled as well and that's what worked for me. |
So in the fix from this code, the |
If that's the case, then it would work. I thought it was a different option, but I thought wrong. My apologies |
If that doesn't work when we merge this in, open up a bug and I'll look. |
kind = "string", | ||
allowed = { | ||
"On", | ||
"Off" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using the boolean
type would probably be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about using those, but there are also plenty of spots in the code where "Off" and "On" are used in place of a boolean value. Should we start just using boolean instead? If so, that's an easy fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The inputs for boolean are more flexible:
Lines 762 to 787 in 561f9a9
local mapping = { | |
["false"] = false, | |
["no"] = false, | |
["off"] = false, | |
["on"] = true, | |
["true"] = true, | |
["yes"] = true, | |
} | |
if type(value) == "string" then | |
value = mapping[value:lower()] | |
if value == nil then | |
error { msg="expected boolean; got " .. value } | |
end | |
return value | |
end | |
if type(value) == "boolean" then | |
return value | |
end | |
if type(value) == "number" then | |
return (value ~= 0) | |
end | |
return (value ~= nil) |
From memory there are issues when using this with look up tables, which is how the toolsets work. So, it's up to you whether you think it's a good idea or not.
kind = "string", | ||
allowed = { | ||
"On", | ||
"Off" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
did this got implemented in a new release already? |
No new release has been cut since this. You can either use binaries from a recent GitHub Action, or you could build premake yourself |
nevermind - i just noticed it will be in Premake 5.0.0 beta 3 |
What does this PR do?
Closes #2105 by introducing 2 new APIs. Introduces
buildstlmodules
andenablemodules
to thevsXXXX
exporters.buildstlmodules
controls if the STL modules should be built for C++23 andenablemodules
controls if modules are to be built. This API does not affect if module files are added to the project or not.Example Usage:
How does this PR change Premake's behavior?
Does not affect existing scripts.
Anything else we should know?
N/A
Did you check all the boxes?
closes #XXXX
in comment to auto-close issue when PR is merged)You can now support Premake on our OpenCollective. Your contributions help us spend more time responding to requests like these!