Runtime: Provided a CoreFX API Analyzer

Created on 5 Jan 2018  路  6Comments  路  Source: dotnet/runtime

Rationale

There are a number of APIs in CoreFX for which special rules may apply, or for which it may be useful to provide additional diagnostics at compile time or during development time.

As such, I propose we create a dedicated project (or repository) to contain these CoreFX specific analyzers.

Example

The new Hardware Intrinsic APIs occasionally have special rules. For example, Vector128<float> System.Runtime.Intrinsics.X86.Sse.Shuffle(Vector128<float>, Vector128<float>, byte) requires that the third parameter be a constant so that the underlying hardware instruction can be emitted appropriately.

The diagnostic that applies to this is very niche, and will only realistically be used by the hardware intrinsic APIs, so doing something like providing a language feature to enforce it is unlikely to happen.

However, implementing a diagnostic analyzer that detects the case where a user does not pass a constant and therefore violates the API contract is fairly trivial and greatly improves the development experience when consuming said API.

Other analyzers could include common bug fixes made to the repos or suggestions to use the latest APIs that are being added. For example, many of the Desktop framework APIs only took a string. However, CoreCLR added a number of overloads which allow passing single characters instead. Additionally, CoreFX has recently added a number of overloads which take Span<T>. Both of these are code-fix analyzers that could be easily suggested to the developer.

area-Meta documentation

Most helpful comment

.NET Core specific analyzers live under https://github.com/dotnet/roslyn-analyzers/tree/master/src/Microsoft.NetCore.Analyzers already.

We have zero references (or at least zero that I could find) on CoreFX or CoreCLR that indicate that exists, but if we already have a package/project it would be good to continue using it and simply add documentation so people can find it.

All 6 comments

As an example of how trivial some of the analyzers can be to implement, here is a basic prototype for const enforcement for Sse.Shuffle:
HardwareIntrinsicMustUseConstAnalyzer.cs.txt

.NET Core specific analyzers live under https://github.com/dotnet/roslyn-analyzers/tree/master/src/Microsoft.NetCore.Analyzers already. How would this differ? E.g. How do we decide when an analyzer should go the the existing location vs. the new location?

.NET Core specific analyzers live under https://github.com/dotnet/roslyn-analyzers/tree/master/src/Microsoft.NetCore.Analyzers already.

We have zero references (or at least zero that I could find) on CoreFX or CoreCLR that indicate that exists, but if we already have a package/project it would be good to continue using it and simply add documentation so people can find it.

Docs: https://github.com/dotnet/roslyn-analyzers/blob/master/src/Microsoft.NetCore.Analyzers/Microsoft.NetCore.Analyzers.md, maybe link it in CorxFX repo's docs?

There should probably be a link and more explicit documentation in all three repos (CoreFX, CoreCLR, and Roslyn-Analyzers) that the Microsoft.NetCore.Analyzers package is specifically for CoreFX API diagnostics.

How do we decide when an analyzer should go the the existing location vs. the new location?

IMO one of the deciding properties of analyzer for locating it in some repo is its usage scope. In the case of HW intrinsics analyzer it will work only with the code which references System.Runtime.Intrinsics.XX and self explanatory name could be System.Runtime.Intrinsics.Analyzer. It should live in System.Runtime.Intrinsics or architecture specific directory (System.Runtime.Intrinsics.X86, System.Runtime.Intrinsics.Arm) as a separate tool project, which should be obligatory to distribute packaged with netcoreapp but loaded, referenced by project and used only when System.Runtime.Intrinsics.XX reference assembly is used. Having it in separate repo would create unnecessary build dependency for corefx and coreclr.

Was this page helpful?
0 / 5 - 0 ratings