roslyn
roslyn copied to clipboard
[New API] stackalloc in IOperation
Background and Motivation
I have a collection of Roslyn analyzers that were previously using the syntax and semantic level to register actions. After some guidance from a Roslyn team member, I've nearly finished the process of changing them all to IOperation instead. One of my holdouts is UnboundedStackallocAnalyzer which still registers on SyntaxKind.StackAllocArrayCreationExpression.
I would like to convert this to an IOperation-based analyzer instead so I can improve uniformity in my codebase, as well as making use of the much more ergonomic APIs exposed through IOperation compared to SyntaxKind.
Proposed API
namespace Microsoft.CodeAnalysis.Operations
{
public interface IArrayCreationOperation
{
+ public bool IsStackAlloc { get; }
}
Usage Examples
See https://github.com/Vannevelj/SharpSource/blob/648fc377db5747e1517f162884b095d07cebdf2e/SharpSource/SharpSource/Diagnostics/UnboundedStackallocAnalyzer.cs#L30 for the original code that would be converted into IOperation.
Alternative Designs
IStackAllocOperation
We can create a new operation kind:
namespace Microsoft.CodeAnalysis.Operations
{
+ public interface IStackAllocOperation
+ {
+ public IOperation Operation { get; }
+ }
I decided against this because I imagine in most cases we'll want to treat stackalloc arrays the same as regular ones, for the purpose of static analysis. After all, if we wanted to treat them differently then we'd have had to jump through hoops until now and a feature request would've come about earlier. By introducing a property on IArrayCreationOperation we avoid fragmentation.
Stackalloc vs StackAlloc
The keyword is stackalloc but the syntax node is StackAllocArrayCreationExpression so I figured StackAlloc is the capitalization to use. Alternative would be IsStackalloc or IStackallocOperation.
Risks
It is my understanding that stackalloc can only be used in combination with arrays today. If that is incorrect or if that is likely to change in the future, an IStackAllocOperation might be more prudent after all. However if we don't expect its use cases to expand significantly, then we can always add an IsStackAlloc property onto those operations in the future.
@333fred Where do you want to route this? Not sure which area path this should live in.
See also https://github.com/dotnet/roslyn/issues/20123
I'll let @AlekseyTs chime in as well, but I don't believe that we should be reusing IArrayCreationOperation here. I don't really buy the argument that stackalloc will want to be treated like array creation most of the time. stackalloc has a lot of additional restrictions and semantics that array creations don't have, and imo most analyses and refactorings do not apply to both.
I am also leaning towards special IOperation node for stackalloc expressions
Hello. +1 request here, and willing to implement.
@mavasani can't we move it to approved yet?
@B1Z0N the original poster didn't update their post to address our feedback to have a dedicated node be the regular proposal. If you want to open a new proposal addressing this feedback, we can move forward.
It will also need to be more complete, and show how to handle initializers and dimensions. I get that that's why the original request reused ArrayCreation, but they're not really related.