Limit tracked exposed WinRT interfaces to 128 #2232
+86
−5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Introduce TryAddExposedInterfaceType to centralize adding tracked projected Windows Runtime interfaces and enforce a hard cap of 128 entries. Replace direct interfaces.Add calls with the helper so discovery will emit WellKnownInteropExceptions.ExceededNumberOfExposedWindowsRuntimeInterfaceTypesWarning (id 84) and stop adding further interfaces when the limit is reached. Adds the new warning factory in WellKnownInteropExceptions and preserves existing warning-as-error behavior via args.TreatWarningsAsErrors.
There was a problem hiding this comment.
Pull request overview
Adds a hard cap to the number of projected WinRT interfaces tracked for exposed types to prevent runaway discovery, and introduces a warning when the limit is hit.
Changes:
- Adds a new well-known warning (ID 84) for exposed types exceeding the tracked interface limit.
- Updates interop type discovery to stop tracking additional interface types once 128 have been collected.
- Refactors repeated interface-add logic into a helper method.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/WinRT.Interop.Generator/Errors/WellKnownInteropExceptions.cs | Adds a new warning for exceeding the max tracked WinRT interface count. |
| src/WinRT.Interop.Generator/Discovery/InteropTypeDiscovery.cs | Enforces the 128-interface cap during discovery via a new helper method. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/WinRT.Interop.Generator/Errors/WellKnownInteropExceptions.cs
Outdated
Show resolved
Hide resolved
src/WinRT.Interop.Generator/Errors/WellKnownInteropExceptions.cs
Outdated
Show resolved
Hide resolved
Ensure finalization blocks always run by replacing loop breaks with explicit goto labels (FinalizeUserDefinedType and FinalizeSzArrayType) in InteropTypeDiscovery. Update TryAddDiscoveredInterface to short-circuit when an interface is already present and to explicitly ignore the builder Add return value. Change TypeSignatureEquatableSet.Builder.Add to return a bool (forwarding the underlying set.Add result) and add a Contains method so callers can check membership before adding. These changes fix control-flow gaps that skipped finalization logic and improve correctness when handling duplicate interface discoveries.
Correct a typo in src/WinRT.Interop.Generator/Errors/WellKnownInteropExceptions.cs XML summary comment: changed 'limited' to 'limit'. This is a documentation-only change with no functional impact.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Title. Not 100% sure there can't be some edge cases where we cap the interfaces but then some code is invalid because it relies on some generic instantiations not yet discovered, but then again this shouldn't really ever be hit in the first place.