Add ILLink annotations to S.D.Common related to DbConnectionStringBuilder #54280
Conversation
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
|
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
src/libraries/System.Data.Common/src/System/Data/Common/DbConnectionStringBuilder.cs
Show resolved
Hide resolved
src/libraries/System.Data.Common/tests/TrimmingTests/DbConnectionStringBuilder.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.ComponentModel.TypeConverter/ref/System.ComponentModel.TypeConverter.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Data.Common/tests/TrimmingTests/DbConnectionStringBuilder.cs
Outdated
Show resolved
Hide resolved
marek-safar
added
the
linkable-framework
|
Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr Issue Details
|
src/libraries/System.Data.Common/src/System/Data/Common/DbConnectionStringBuilder.cs
Outdated
Show resolved
Hide resolved
| return TypeDescriptor.GetComponentName(this, true); | ||
| } | ||
| [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", | ||
| Justification = "The type of component is statically known. This class is marked with [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)]")] |
There was a problem hiding this comment.
Does GetAttributes() need a Type thisType = GetType() call as well? Since it has the same justification as the other 3?
There was a problem hiding this comment.
the tests pass (even when I remove other tests which call GetType) and per @vitek-karas's comment they don't trim attributes other than some set of "safe to trim" attributes
| protected virtual void GetProperties(Hashtable propertyDescriptors) | ||
| { | ||
| long logScopeId = DataCommonEventSource.Log.EnterScope("<comm.DbConnectionStringBuilder.GetProperties|API> {0}", ObjectID); | ||
| try | ||
| { | ||
| // Below call is necessary to tell the trimmer that it should mark derived types appropriately. | ||
| // We cannot use overload which takes type because the result might differ if derived class implements ICustomTypeDescriptor. | ||
| Type thisType = GetType(); |
There was a problem hiding this comment.
I would remove this. The method is RequiresUnreferencedCode, we don't make any guarantees that things are going to be preserved if you call it.
There was a problem hiding this comment.
I think it might still be a good idea to at least try to make it work as much as we can even though there is no guarantees
There was a problem hiding this comment.
(or rather doesn't hurt to have it working for some scenarios)
…nitial_config * origin/main: (362 commits) [wasm][debugger] Reuse debugger-agent on wasm debugger (dotnet#52300) Put Crossgen2 in sync with dotnet#54235 (dotnet#54438) Move System.Object serialization to ObjectConverter (dotnet#54436) Move setting fHasVirtualStaticMethods out of sanity check section (dotnet#54574) [wasm] Compile .bc->.o in parallel, before passing to the linker (dotnet#54053) Change PathInternal.IsCaseSensitive to a constant (dotnet#54340) Make mono_polling_required a public symbol (dotnet#54592) Respect EventSource::IsSupported setting in more codepaths (dotnet#51977) Root ComActivator for hosting (dotnet#54524) Add ILLink annotations to S.D.Common related to DbConnectionStringBuilder (dotnet#54280) Fix finalizer issue with regions (dotnet#54550) Add support for multi-arch install locations (dotnet#53763) Update library testing docs page to reduce confusion (dotnet#54324) [FileStream] handle UNC and device paths (dotnet#54483) Update NetAnalyzers version (dotnet#54511) Added runtime dependency to fix the intermittent test failures (dotnet#54587) Disable failing System.Reflection.Tests.ModuleTests.GetMethods (dotnet#54564) [wasm] Move AOT builds from `runtime-staging` to `runtime` (dotnet#54577) Keep obj node for ArrayIndex. (dotnet#54584) Disable another failing MemoryCache test (dotnet#54578) ...
ghost
locked as resolved and limited conversation to collaborators
No description provided.