-
Notifications
You must be signed in to change notification settings - Fork 201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support implicit blittability for structs declared in and not exposed outside of the current compilation. #1126
Support implicit blittability for structs declared in and not exposed outside of the current compilation. #1126
Conversation
… outside of the current compilation.
…elab into internal-implicit-blittable
public static bool HasOnlyBlittableFields(this ITypeSymbol type) => HasOnlyBlittableFields(type, new HashSet<ITypeSymbol>(SymbolEqualityComparer.Default)); | ||
#pragma warning restore RS1024 // Compare symbols correctly | ||
|
||
private static bool HasOnlyBlittableFields(this ITypeSymbol type, HashSet<ITypeSymbol> seenTypes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do me a favor here and pass a recursion counter - when it reaches 0 we throw. Alternatively this could be converted to use a Stack<T>
and turned into an iterative solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same protection for IsConsideredBlittable
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we discussed offline, I think we should leave Stack-overflow protection due to too many nested structs to the Roslyn compiler infrastructure.
…e sure we're handling our recursion check correctly.
070f1e7
to
505a2a2
Compare
… outside of the current compilation. (dotnet/runtimelab#1126) Commit migrated from dotnet/runtimelab@51d9ad8
Also significantly increase our test coverage for generic blittable types as they're another area nearby and something I hadn't added a ton of tests for originally.
Fixes #1125