Fix build warnings and update compiler #3400
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3400 +/- ##
=======================================
Coverage 95.21% 95.21%
=======================================
Files 993 993
Lines 226241 226241
Branches 14524 14524
=======================================
Hits 215426 215426
Misses 9186 9186
Partials 1629 1629 |
| @@ -44,7 +44,7 @@ public override void Initialize(AnalysisContext analysisContext) | |||
|
|
|||
| analysisContext.RegisterCompilationStartAction(startContext => | |||
| { | |||
| var instantiatedTypes = new ConcurrentDictionary<INamedTypeSymbol, object?>(); | |||
| ConcurrentDictionary<INamedTypeSymbol, object?> instantiatedTypes = new ConcurrentDictionary<INamedTypeSymbol, object?>(); | |||
There was a problem hiding this comment.
Those explicit types were not necessary, were they?
There was a problem hiding this comment.
The commit introducing these changes is named "Use explicit type where compiler fails to evaluate nullability" so I think they matters.
There was a problem hiding this comment.
@jcouv yes, they are necessary. var doesn't work with local functions, at least not here.
There was a problem hiding this comment.
Do we need to file a compiler bug?
There was a problem hiding this comment.
I think there are already bugs filed for improving local function support. It hit Roslyn too.
There was a problem hiding this comment.
...Analyzers/Core/Microsoft.NetCore.Analyzers/Runtime/DoNotPassLiteralsAsLocalizedParameters.cs
Show resolved
Hide resolved
| @@ -66,14 +66,14 @@ public sealed override void Initialize(AnalysisContext context) | |||
| return; | |||
| } | |||
|
|
|||
| var forwardGraph = new ConcurrentDictionary<ISymbol, ConcurrentDictionary<ISymbol, bool>>(); | |||
| var invertedGraph = new ConcurrentDictionary<ISymbol, ConcurrentDictionary<ISymbol, bool>>(); | |||
| ConcurrentDictionary<ISymbol, ConcurrentDictionary<ISymbol, bool>> forwardGraph = new ConcurrentDictionary<ISymbol, ConcurrentDictionary<ISymbol, bool>>(); | |||
There was a problem hiding this comment.
Please retarget all security rule changes to 2.9.x branch. Also, there is no nullability involved here, so why was this required?
There was a problem hiding this comment.
It's not practical to backport this change. The 2.9.x branch doesn't support this toolchain or compiler version so it would be a large backport of much more than just this pull request.
I don't believe these changes will be a problem in practice. /cc @dotpaul for awareness.
No description provided.