cleanup ErrorUtilities

[danmoseley]

1. Moved resource string existence check (in debug) consistently outside of the condition. This detected cases where we were using an invalid resource string, but the condition was never false. Fixed these. Note, it doesn't need to be localized message or even probably the same exception type because almost by definition, these conditions have never been true, and possibly can never be true (otherwise they'd have hit the missing resource already). Also, change method to be debug-only so as to remove the #if.

2. Moved s_throwExceptions consistently inside the condition check as cleaner.

3. Move exception construction consistently out of the VerifyThrowXX methods so this cold path doesn't prevent them inlining.

4. Remove string formatting inside the condition in one place where string interpolation was happening.

[JanKrivanek]

Thank you!

[danmoseley]

This would make it hard to keep the two strings in alignment, right? Is there a plan there?

I can put them into Shared\Resources\Strings.shared.resx if you like? that is built into tasks, engine, utilities, task host, and exe. but then they are only in one file in the code.

[Forgind]

This would make it hard to keep the two strings in alignment, right? Is there a plan there?

I can put them into Shared\Resources\Strings.shared.resx if you like? that is built into tasks, engine, utilities, task host, and exe. but then they are only in one file in the code.

Works for me! Maybe get buyoff from someone who actually works on MSBuild, though 😅

[danmoseley]

What do you work on @Forgind ... entirely in SDK?

[Forgind]

Yup!

[JanKrivanek]

This would make it hard to keep the two strings in alignment, right? Is there a plan there?

I can put them into Shared\Resources\Strings.shared.resx if you like? that is built into tasks, engine, utilities, task host, and exe. but then they are only in one file in the code.

Works for me! Maybe get buyoff from someone who actually works on MSBuild, though 😅

Sounds good.

I suppose this string would not change often (especially to extent of adding/removing fmting params) - but I definitely wouldn't complain to future-proof it by moving to single resx

[rokonec]

Thanks for taking time to clean it.

[danmoseley]

feedback hopefully addressed

[danmoseley]

(I removed some low value comments and formatting for todays less narrow monitors..)

[danmoseley]

is this ready to merge?