CA1810 Initialize reference type static fields inline#7179
CA1810 Initialize reference type static fields inline#7179AR-May merged 17 commits intodotnet:mainfrom
Conversation
src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupLoggingHelper.cs
Show resolved
Hide resolved
Co-authored-by: Forgind <Forgind@users.noreply.github.com>
|
There are still 3 occurrences that I believe are false positives. I can add a suppression, but I think if this takes a while to merge it might be fixed in the analyzer by then. |
Forgind
left a comment
There was a problem hiding this comment.
Looks good! I'm assuming the false positive problem isn't about to be resolved, though we also shouldn't be merging right now because our last insertion failed for some reason, so we have a little time.
Co-authored-by: Forgind <Forgind@users.noreply.github.com>
There is only one false positive and it has a suppression now. The other two were me not fully reading the analyzer article and understanding what it wanted me to do. Hence the fixes I just submitted. |
ladipro
left a comment
There was a problem hiding this comment.
Please note the tradeoffs being made when switching from an explicit static ctor to inline static field initializion. As the analyzer suggests, with beforefieldinit the JITted code may be slightly faster because it doesn't need to execute init checks. It may however lead to unwanted eager initialization. We should be especially careful when dealing with expensive initialization code. If there are scenarios where MSBuild "uses" the type but does not really call it or access its fields, then expensive initialization should be left in static ctors.
src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupLoggingHelper.cs
Show resolved
Hide resolved
|
|
||
| // keep this up-to-date; always point to the latest visual studio version. | ||
| internal static readonly Version visualStudioVersionLatest = visualStudioVersion160; | ||
| internal static readonly Version visualStudioVersionLatest = visualStudioVersion170; |
| /// </summary> | ||
| #pragma warning disable CA1810 // Initialize reference type static fields inline | ||
| static MSBuildApp() | ||
| #pragma warning restore CA1810 // Initialize reference type static fields inline |
There was a problem hiding this comment.
nit: I think I'd slightly prefer putting the restore below the end of this function, but I don't care too much.
Relates to #7174