Mark ternary macros that can be evaluated as constants as const

[ThadHouse]

Previously, all conditionals were marked as not constant. However, if the Cond, LHS and RHS are all constant, the ternary can be marked constant as well

[ThadHouse]

Is there a better way to do this?

[tannergooding]

Not really, this is about how everything works for validating diagnostics.

[tannergooding]

nit: This isn't quite right. If IsConstant(conditionalOperator.Cond) is true and the evaluated value of Cond is true, then only IsConstant(conditionalOperator.LHS) needs to be true.

[ThadHouse]

a ? b :c does that mean Cond is a and LHS is b : c? Otherwise confused by why only 2 of the 3 would need to be true.

[ThadHouse]

This code doesn't check that Cond evaluates to true. It just checks that it evaluates to a constant.

[tannergooding]

cond ? LHS : RHS

x = cond ? LHS : RHS is basically shorthand for:

if (cond)
{
    x = LHS;
}
else
{
    x = RHS;
}

So if you have x = trueCond ? 5 : sqrt(globalValue), then you want it to still evaluate to be "constant". As it is now, it won't since IsConstant(conditionalOperator.RHS) will return false

[tannergooding]

Likewise if you had x = falseCond ? sqrt(globalValue) : 5, you want it to evaluate as constant while IsConstant(conditionalOperator.LHS) will be the only thing to return false

[ThadHouse]

Since the generator actually generates the ternary even if the Cond is constant, wouldn't the current impl need both LHS and RHS to be true? I agree if the generator actually lowered the ternary then only the evaluated condition would need to be checked, but thats not what gets generated currently.

[tannergooding]

Looks like yes actually. I thought C# would drop the dead code path here, but looks like its not.

[ThadHouse]

Yeah. I agree if the generator is changed to lower everything, then this evaluation could be more fine grained. But for now, it will generate the whole thing, so all 3 need to be constant evaluated in order to generate const.

[tannergooding]

Thanks, this looks good and I'll merge once CI finishes

[ThadHouse]

Anything we can do to poke the windows build thats been stuck for a few weeks?

[tannergooding]

It actually has passed and the status never got updated on the GitHub side. I'll force merge.