Move ArrayBuilder.GetInstance helpers to Microsoft.CodeAnalysis.PooledObjects package #76971
Conversation
dotnet-issue-labeler
bot
added
Area-Compilers
untriaged
| @@ -8,9 +8,7 @@ | |||
| namespace Microsoft.CodeAnalysis.PooledObjects; | |||
|
|
|||
| [NonCopyable] | |||
There was a problem hiding this comment.
Adds [NonCopyable] attribute to the type definition.
There was a problem hiding this comment.
What about just requiring the consumer to provide this attribute in all cases?
There was a problem hiding this comment.
Isn't the analyzer Roslyn specific? I would rather not impose more requirements on consumers of this source package.
That said, I'm working on another source package (say Microsoft.CodeAnalysis.Contracts) with basic contracts and exception utilities, which are used by our threading utilities (extracted to source package #76937).
We can include NonCopyable there. It's a contract after all, so it would fit.
There was a problem hiding this comment.
#76997 (Add Microsoft.CodeAnalysis.Contracts source package)
| @@ -720,5 +720,24 @@ public ImmutableArray<S> SelectDistinct<S>(Func<T, S> selector) | |||
| set.Free(); | |||
| return result.ToImmutableAndFree(); | |||
| } | |||
|
|
|||
| public static PooledDisposer<ArrayBuilder<T>> GetInstance(out ArrayBuilder<T> instance) | |||
There was a problem hiding this comment.
Definitely need compiler sign off here. This will maje some unusable members show up in completion for a very common type.
There was a problem hiding this comment.
Why is this an unusable member?
There was a problem hiding this comment.
Perhaps instead of banning the API we can wrap these in #if !COMPILERCORE . If we do, it'd be better to rename the symbol to something more descriptive, e.g. ROSLYN_COMPILER_CORE as it would appear in any repo consuming the source package.
@dotnet/roslyn-compiler Which approach do you prefer?
These helpers are generally useful to consumers of the package. Especially when combining Roslyn source packages - see #76937
They were defined in Workspace layer since we do not want to allow using them in the compiler (due to perf concerns when used widely). To keep this intentional constraint the change adds them to compiler-specific banned API list.