/O2 is incompatible with /RTC1

[tvandijck]

No description provided.

[starkos]

This one fails several unit tests due to the changed behavior.

[TurkeyMan]

It would help to rebase all these PR's on latest so they pipe through travis.

[tvandijck]

I'll give that a shot...

[starkos]

This one is still failing several of the unit tests due to the flag change.

[tvandijck]

yes, do you want me to fix the tests? or do we just not want this one?
basically what I'm getting in unit-tests is a couple of extra "DefaultRuntime" flags here and there.. nothing harmless really, and from visual studio's perspective not a behavior change.

[starkos]

Just need the tests to pass with the new behavior.

[tvandijck]

All fixed...

[starkos]

I'm getting pushback on this change. We have projects that have been building fine for months now getting new <BasicRuntimeChecks>Default</BasicRuntimeChecks> markup added, and there doesn't seem to be a good reason for it. I seem to recall a thread somewhere about why this change was necessary but I can't find it now. Visual Studio seems to work just fine without this value set; should be done conditionally in project scripts instead?

filter "Release"
   flags "NoRuntimeChecks"

[tvandijck]

Well, I really want to stay away from flags as much as possible... they are so hard to 'override' for projects and can only be removed to revert to the default behaviour, which is sometimes exactly NOT what I want....

anyway, this thing fixes the following error:
1>cl : Command line error D8016: '/O2' and '/RTC1' command-line options are incompatible

So if you create a combination of RuntimeChecks, and /O2 (optimized build), your project simply does not build. Arguably this can be fixed with a filter or some other setting in premake, but my argument is that premake should NEVER EVER create combinations that are invalid.

I'm all for "fixing" this in a better way that does not generate the extra output if it's not needed, but I do like to have this fix.

[starkos]

anyway, this thing fixes the following error:
1>cl : Command line error D8016: '/O2' and '/RTC1' command-line options are incompatible

We're not seeing this issue here. I will investigate further and see if I can understand why.

[tvandijck]

what version of visual studio? I'm seeing this with 2013...

[TurkeyMan]

I think it's VS version specific.

So if you create a combination of RuntimeChecks, and /O2 (optimized build), your project simply does not build.

Isn't this something that should be caught in a validation step somewhere? We do need a mechanism to perform/extend the validation process in the bake.
Or would you want premake to attempt to correct the situation intelligently, rather than emit an error detailing invalid build configuration?

[tvandijck]

Isn't this something that should be caught in a validation step somewhere?

No, I don't think you can... because this is unique to visual studio. Maybe clang does in fact allow for this in optimized builds...

Or would you want premake to attempt to correct the situation intelligently,

that is what I attempted to do with the commit. In the visual studio action, I ignore the runtime check setting for optimized builds..

[starkos]

Okay, I've been poking at this one a bit and have some findings…

If I manually create a sample C++ project with VS 2013, it does not contain any BasicRuntimeChecks elements in the project XML. If I open the project in VS and bring up the project properties, I see that Basic Runtime Checks is set to "Both" (/RTC1) for the debug build, and "Default" (no flag) for the release build.

So I tried this Premake script:

solution "MySolution"
    configurations { "Debug", "Release" }

filter "Debug"
    flags "Symbols"

filter "Release"
    optimize "speed"

project "MyProject"
    kind "ConsoleApp"
    files "hello.cpp"

I get the same result: no BasicRuntimeChecks element in the project file, "Both" for the debug build, "Default" for the release build, /O2 optimization and everything builds fine.

So here's the $1M question: since there iares no BasicRuntimeChecks element in the project file, what criteria is VS using to determine when to turn them on? If I could figure that out, I could apply a narrower test and only write the element when it was really needed. (Or: are you somehow adding that element through a customization of your own? Because the public version of Premake has no way to set that value to anything other than "Default").

Would it be possible to get an example project that demonstrates the problem, if I were to reverse this change? I could then compare the settings to my default project and figure out what is driving VS to turn on /RTC1.

[starkos]

Oh, and per your previous point…

really want to stay away from flags as much as possible... they are so hard to 'override'

…this seems like a really good candidate to move to an API call. VS supports multiple values, so it really isn't the toggle that the flag would imply anyway.

[tvandijck]

Here is a repo:

solution "MySolution"
    configurations { "DebugSlow", "DebugFast", "Release" }

filter "Debug*"
    flags "Symbols"

filter "DebugFast"
    optimize "speed"
    runtime  "debug"

filter "Release"
    optimize "speed"

project "MyProject"
    kind "ConsoleApp"
    files "hello.cpp"

[tvandijck]

and just to clarify, the "DebugFast" config is where it goes wrong.... since it takes on the default visual studio properties from the "debug" config, but we're doing an optimized build with the debug runtime.

the debug runtime is the important part here, because of this little piece in Microsoft.Cl.Common.props

<ItemDefinitionGroup Condition="'$(UseDebugLibraries)' == 'true'">
    <ClCompile>
      <RuntimeLibrary                   Condition="'%(ClCompile.RuntimeLibrary)'                == ''">MultiThreadedDebugDll</RuntimeLibrary>
      <BasicRuntimeChecks               Condition="'%(ClCompile.BasicRuntimeChecks)'            == ''">EnableFastChecks</BasicRuntimeChecks>

[tvandijck]

given that actually, I think what we could do is:

if cfg.flags.NoRuntimeChecks or (config.isOptimizedBuild(cfg) and runtime=="Debug") then

since that is ultimately the entire condition under which it fails.