Skip to content
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

Resolve SizeParamIndex to a TypePositionInfo during MarshallingInfo parsing #1293

Conversation

jkoritzinsky
Copy link
Member

Move the resolution/validation of SizeParamIndex up to MarshallingAttributeInfo.

Enables us to remove the concept of mapping an "index" -> element name/TypePositionInfo from the context and make it only part of the attribute parser. Since this mapping only makes sense in certain scenarios, this allows custom contexts to not have to provide overrides of the GetTypePositionInfoForManagedIndex method that either always return null or delegate to a parent context.

@jkoritzinsky jkoritzinsky added the area-DllImportGenerator Source Generated stubs for P/Invokes in C# label Jul 2, 2021
// This ensures that we've unwound the whole cycle so when we return, there will be no cycles in the count info.
catch (CyclicalCountElementInfoException ex) when (ex.StartOfCycle == param.Name)
{
_diagnostics.ReportConfigurationNotSupported(attrData, $"Cyclical {ManualTypeMarshallingHelper.MarshalUsingProperties.CountElementName}");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should Cyclical {0} be a resource string?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably. I’ll do a follow-up PR to clean up how diagnostics are reported by the marshalling info parser to be a bit cleaner.

@elinor-fung
Copy link
Member

Are there existing tests for the cyclical count info failure case?

@jkoritzinsky jkoritzinsky merged commit eea0bae into dotnet:feature/DllImportGenerator Jul 7, 2021
@jkoritzinsky jkoritzinsky deleted the paramindex-early-resolution branch July 7, 2021 21:38
runtimelab-bot pushed a commit that referenced this pull request Aug 4, 2021
* Add SslApplicationProtocol.Http3

Fix #1293

* Add XML doc

* Reuse in Http3Connection

* Reuse in tests

* HTTP/3 has no negotiation

* Use SslApplicationProtocol.Http3.ToString() in tests

* Ascending order in ref
jkoritzinsky added a commit to jkoritzinsky/runtime that referenced this pull request Sep 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-DllImportGenerator Source Generated stubs for P/Invokes in C#
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants