Remove conditional-evaluation-order warning

[rainersigwald]

The warning

MSB4130: The condition "{0}" may have been evaluated incorrectly in an earlier version of MSBuild. Please verify that the order of the AND and OR clauses is written as intended. To avoid this warning, add parentheses to make the evaluation order explicit.

is emitted whenever a condition has an and and an or in parallel, for example X and Y or Z.

Looking through ancient history, it appears to have been introduced in MSBuild 3.5 in 2007, because a bug in MSBuild 2.0 that caused unexpected evaluation order was fixed then.

It feels wrong to emit an informational-only warning about a behavior change that happened 10 years ago.

Eliminating the check for this warning would allow pruning a tree of methods that exist only to provide it. They're currently in the code in REMOVE_COMPAT_WARNING regions. I propose to do just what that implies.

[rainersigwald]

cc @onovotny who pointed this out offline.

[clairernovotny]

So I guess I should tell people to ignore those warnings, document them as a known issue and point to this issue?

[rainersigwald]

@onovotny Since you're writing "library" code I'd recommend (over-)parenthesizing to make the warning go away for everyone. It's hard to predict a timeline on when this change might be made.

[clairernovotny]

Thanks -- I'll take another stab based on your offline reply and try to fix it. Might ping you again for more help as I'm getting lost in all of those parens 😁

[rainersigwald]

It was a real pain figuring out what was going on, even in a debugger.

I think I've got the hang of it now though: set a breakpoint on the warning, and when it breaks in, walk the node object to find a unique string from the condition. Don't be fooled like I was and look at expression--that always has the whole expression, not the sub-part that's causing the warning.

(Just in case someone wanders into this bug with a similar issue and a gnarly expression.)

[clairernovotny]

There's a debugger for this?

[rainersigwald]

No, debugging MSBuild.exe.

[clairernovotny]

Yeah, wishful thinking. A debugger for the targets themselves would be great!

[danmoseley]

I agree we should remove this, I haven't heard a single customer mentoin it in 10 years. Or notice the problem before we had the warning.

[danmoseley]

@onovotny for debugger, feel free to +1 my suggestion here #291.