[C#] Fix trim warnings #9182

JamesNK · 2021-11-02T21:59:16Z

Background:
.NET 6 introduces trim analysis and warnings. When Google.Protobuf is included in an app, and trimming is enabled, the app will get a list of trim-related warnings from Google.Protobuf. These are typically places in code that reflection is used but the type could have been trimmed at build time. Use those areas of code could fail after trimming at runtime.

For more information, see https://docs.microsoft.com/en-us/dotnet/core/deploying/trimming/prepare-libraries-for-trimming

This PR fixes the trim warnings for Google.Protobuf by adding trimming annotations. They either preserve type information so the trimmer can make better decisions, or warn the caller of Google.Protobuf that the API is not compatible with trimming.

I tested all trim warnings are fixed by using Google.Protobuf with a test app in the grpc-dotnet repo.

Also, I updated the test projects to use netcoreapp3.1. The previous target, netcoreapp2.1, is no longer supported and build was generating warnings.

cc @jtattermusch @captainsafia

agocke · 2021-11-03T05:13:21Z

I took a look and I think I found a few more problematic areas: #9183

JamesNK · 2021-11-03T18:47:12Z

How did you find those?

I tested build warnings with RC1. Were more warnings added in RC2 and final?

agocke · 2021-11-03T19:31:02Z

I tested by trimming from the LinkedTestClient in the dotnet-grpc project, with the package references changed to project references. I also didn't set IsTrimmable until later, because not setting IsTrimmable will scan the whole assembly, just in case.

JamesNK · 2021-11-04T09:41:40Z

@MichalStrehovsky @agocke @jkotas All warnings fixed.

The one thing I'm unsure about is this code:

protobuf/csharp/src/Google.Protobuf/JsonFormatter.cs

Lines 884 to 898 in 7ccf4d8

    
           var enumType = value.GetType(); 
        
           Dictionary<object, string> nameMapping; 
        
           lock (dictionaries) 
        
           { 
        
               if (!dictionaries.TryGetValue(enumType, out nameMapping)) 
        
               { 
        
                   nameMapping = GetNameMapping(enumType); 
        
                   dictionaries[enumType] = nameMapping; 
        
               } 
        
           } 
        
           string originalName; 
        
           // If this returns false, originalName will be null, which is what we want. 
        
           nameMapping.TryGetValue(value, out originalName); 
        
           return originalName;

Enum types are specified in code-generated descriptor code here -

protobuf/csharp/src/Google.Protobuf/Reflection/GeneratedClrTypeInfo.cs

Lines 81 to 91 in 7ccf4d8

    
           /// <summary> 
        
           /// The CLR types for enums within this file/message descriptor. 
        
           /// </summary> 
        
           public Type[] NestedEnums { get; } 
        
           /// <summary> 
        
           /// Creates a GeneratedClrTypeInfo for a message descriptor, with nested types, nested enums, the CLR type, property names and oneof names. 
        
           /// Each array parameter may be null, to indicate a lack of values. 
        
           /// The parameter order is designed to make it feasible to format the generated code readably. 
        
           /// </summary> 
        
           public GeneratedClrTypeInfo(Type clrType, MessageParser parser, string[] propertyNames, string[] oneofNames, Type[] nestedEnums, Extension[] extensions, GeneratedClrTypeInfo[] nestedTypes)

- unfortunately it is an array of Type, so the dynamic members attribute isn't allowed. Have you considered allowing that attribute on Type[]?

csharp/src/Google.Protobuf/Compatibility/TypeExtensions.cs

csharp/src/Google.Protobuf/JsonFormatter.cs

agocke · 2021-11-04T17:37:15Z

Unfortunately the problem with arrays isn't really that we couldn't allow the attribute, it's that the attribute represents the ability to track the variable, and tracking the individual elements of an array would probably be too difficult/cost prohibitive.

That said, an improvement to investigate for the future.

JamesNK · 2021-11-05T08:08:57Z

csharp/src/Google.Protobuf/Reflection/CustomOptions.cs

@@ -77,6 +80,7 @@ internal CustomOptions(IDictionary<int, IExtensionValue> values)
        /// <param name="field">The field to fetch the value for.</param>
        /// <param name="value">The output variable to populate.</param>
        /// <returns><c>true</c> if a suitable value for the field was found; <c>false</c> otherwise.</returns>
+        [RequiresUnreferencedCode(UnreferencedCodeMessage)]


CustomOptions is obsolete so I just marked its methods as incompatible with trimming instead of finding a way to fix it.

JamesNK · 2021-11-05T08:09:58Z

csharp/src/Google.Protobuf/Reflection/SingleFieldAccessor.cs

-                    clrType == typeof(string) ? ""
-                    : clrType == typeof(ByteString) ? ByteString.Empty
-                    : Activator.CreateInstance(clrType);
+                object defaultValue = GetDefaultValue(descriptor);


This is the one notable runtime change. A default value is returned based on the field type instead of using reflection.

JamesNK · 2021-11-05T08:11:24Z

csharp/src/Google.Protobuf/Compatibility/DynamicallyAccessedMemberTypes.cs

+#endregion
+
+#if !NET5_0_OR_GREATER
+// Copied with permission from https://github.com/dotnet/runtime/tree/8fbf206d0e518b45ca855832e8bfb391afa85972/src/libraries/System.Private.CoreLib/src/System/Diagnostics/CodeAnalysis


Add copies of these code analysis attributes for framework versions that don't have them.

Avoids the need to add #ifdef everywhere a code analysis attribute is used.

jtattermusch · 2021-11-09T12:49:10Z

You'll need to update

protobuf/csharp/install_dotnet_sdk.ps1

Line 19 in 13d559b

&$InstallScriptPath -Version 2.1.802

and

protobuf/kokoro/linux/dockerfile/test/csharp/Dockerfile

Line 35 in 13d559b

./dotnet-install.sh --version 2.1.802 && \

(and you'll need e.g. me to push that dockerfile for you).

Ideally, we'd have a separate PR that upgrades the pre-installed SDKs and switches to netcoreapp3.1 targets (they won't run unless you have 3.1 runtime installed), after which we could look into the actual changes in this PR.

JamesNK · 2021-11-09T20:26:44Z

@jtattermusch #9205

jtattermusch · 2021-11-12T10:42:37Z

Let's rebase once #9205 is merged (I'll merge as soon as the test finish).

JamesNK · 2021-11-12T18:52:43Z

Rebased.

jtattermusch · 2021-11-15T09:04:37Z

Seems like tests are failing.

JamesNK · 2021-11-15T21:45:03Z

@jtattermusch Fixed.

jtattermusch · 2021-11-16T09:22:37Z

nit: Makefile.am needs to be updated

++ '[' '!' -z 'csharp/src/Google.Protobuf/Compatibility/UnconditionalSuppressMessageAttribute.cs csharp/src/Google.Protobuf/Compatibility/RequiresUnreferencedCodeAttribute.cs csharp/src/Google.Protobuf/Compatibility/DynamicallyAccessedMembersAttribute.cs csharp/src/Google.Protobuf/Compatibility/DynamicallyAccessedMemberTypes.cs ' ']'
++ echo 'Missing files in EXTRA_DIST: csharp/src/Google.Protobuf/Compatibility/UnconditionalSuppressMessageAttribute.cs csharp/src/Google.Protobuf/Compatibility/RequiresUnreferencedCodeAttribute.cs csharp/src/Google.Protobuf/Compatibility/DynamicallyAccessedMembersAttribute.cs csharp/src/Google.Protobuf/Compatibility/DynamicallyAccessedMemberTypes.cs '
Missing files in EXTRA_DIST: csharp/src/Google.Protobuf/Compatibility/UnconditionalSuppressMessageAttribute.cs csharp/src/Google.Protobuf/Compatibility/RequiresUnreferencedCodeAttribute.cs csharp/src/Google.Protobuf/Compatibility/DynamicallyAccessedMembersAttribute.cs csharp/src/Google.Protobuf/Compatibility/DynamicallyAccessedMemberTypes.cs
++ exit 1

https://source.cloud.google.com/results/invocations/503d13e5-df5f-416e-8974-3b2a19a427ec/log

jtattermusch

A few things I'd like to clarify:

I understand this was tested with a test app with trimming enabled, but it's unclear to me what was the coverage of the test app. IIRC, it might well be true that there are some codepath that could still generate trimming warnings, but the test app hasn't exercised them? So is this PR meant as an incremental improvement that removes many (but possibly not all) trimming issues? Is it even possible to identify all the potential trimming problems at compile time or are the trimming warnings basically heuristics?
how do we make sure new trimming warnings don't get reintroduced in the future? Manually testing with a test app seems quite fragile, and there is currently nothing in this PR that would prevent regressions. If at some point in the future we introduce a new API that has to do with reflection, it seems quite easy to just forget that the API needs to be annotated with the trimming-related attributes.
what is the failure mode if we don't get the trimming related annotations right and someone enables trimming for project that uses Google.Protobuf? Will the resulting application crash because required APIs may have been trimmed? It sounds like the trimming warnings cannot be 100% reliable in the sense "if it builds, it will work".

jtattermusch · 2021-11-15T11:30:49Z

csharp/src/Google.Protobuf/Reflection/GeneratedClrTypeInfo.cs

+        public GeneratedClrTypeInfo(
+            // Preserve all public members on message types when trimming is enabled.
+            // This ensures that members used by reflection, e.g. JSON serialization, are preserved.
+            [DynamicallyAccessedMembers(MessageAccessibility)]


Interesting. Is the trick there that we know that each generated message type will call the
GeneratedClrTypeInfo constructor, and so by piggybacking on GeneratedClrTypeInfo we can make sure that trimming will be avoided for all members of generated proto messages?

if so, I think the comment could do a better job at explaining this

is this really the best place for placing the DynamicallyAccessedMembers attribute?

I can imagine there being use cases where one uses large .proto files (quite common e.g. at google) and only uses a subset of the fields in the message. So one use case to think about is whether trimming can be made to strip all the unused protobuf fields (assuming the user doesn't use reflection on json serialization). I think this is probably quite difficult to achieve, but is something to think about in the future. Also this should be listed as a "limitation" of the current approach?.

is this really the best place for placing the DynamicallyAccessedMembers attribute?

The idea of dynamically accessed members attribute is to annotate those places so Type arguments and generic arguments that are passed to them can be used to include members on those types, without having them explicitly referenced by code. Having GeneratedClrTypeInfo included in code generation seems like a good way to include members on those types that could be used dynamically.

I can imagine there being use cases where one uses large .proto files (quite common e.g. at google) and only uses a subset of the fields in the message. So one use case to think about is whether trimming can be made to strip all the unused protobuf fields (assuming the user doesn't use reflection on json serialization). I think this is probably quite difficult to achieve, but is something to think about in the future. Also this should be listed as a "limitation" of the current approach?.

Including members on types like this could definitely pull in more than you need. There is a tradeoff between everything just working but the app size is larger, and some functionality breaking but smaller apps.

Right now I'm not exactly sure how granular and smart trimming is. For example, imagine a library assembly that has generated types from two large proto files. If your app doesn't use any types from one of the library's proto files, will its static ctor never be evaluated, and therefore GeneratedClrTypeInfo calls are trimmed and so none of its types will be included?

jtattermusch · 2021-11-16T09:37:21Z

FTR since I don't know what the "test app" you tested against was, I ran a small experiment myself.
I tried trimming the Google.Protobuf.Test binary, since Google.Protobuf.Test excercises a large number of Google.Protobuf APIs by definition.
A few tweaks were required (upgrade a few nuget dependencies and delete CustomOptionsTest.cs, which is now "unsupported" for trimming), but I was able to run dotnet publish -r osx-x64 --self-contained -f net60 -p:PublishTrimmed=true Google.Protobuf.Test

That experiment gave me these warnings:

/Users/jtattermusch/github/protobuf/csharp/src/Google.Protobuf.Test/Compatibility/TypeExtensionsTest.cs(113,58): Trim analysis warning IL2077: Google.Protobuf.Compatibility.TypeExtensionsTest.<>c__DisplayClass16_0.<GetMethod_Ambiguous>b__0(): 'target' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods', 'DynamicallyAccessedMemberTypes.NonPublicMethods' in call to 'Google.Protobuf.Compatibility.TypeExtensions.GetMethod(Type,String)'. The field 'System.Type Google.Protobuf.Compatibility.TypeExtensionsTest/<>c__DisplayClass16_0::type' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/Users/jtattermusch/github/protobuf/csharp/src/Google.Protobuf.Test/Google.Protobuf.Test.csproj]
/Users/jtattermusch/github/protobuf/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs(630,13): Trim analysis warning IL2067: Google.Protobuf.JsonFormatterTest.Wrappers_Standalone(Type,Object,String): '#0' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Object System.Activator::CreateInstance(System.Type)'. The parameter 'wrapperType' of method 'Google.Protobuf.JsonFormatterTest.Wrappers_Standalone(Type,Object,String)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/Users/jtattermusch/github/protobuf/csharp/src/Google.Protobuf.Test/Google.Protobuf.Test.csproj]
/Users/jtattermusch/github/protobuf/csharp/src/Google.Protobuf.Test/JsonParserTest.cs(151,13): Trim analysis warning IL2067: Google.Protobuf.JsonParserTest.Wrappers_Standalone(Type,String,Object): '#0' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Object System.Activator::CreateInstance(System.Type)'. The parameter 'wrapperType' of method 'Google.Protobuf.JsonParserTest.Wrappers_Standalone(Type,String,Object)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/Users/jtattermusch/github/protobuf/csharp/src/Google.Protobuf.Test/Google.Protobuf.Test.csproj]
/Users/jtattermusch/github/protobuf/csharp/src/Google.Protobuf.Test/JsonParserTest.cs(152,13): Trim analysis warning IL2067: Google.Protobuf.JsonParserTest.Wrappers_Standalone(Type,String,Object): '#0' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Object System.Activator::CreateInstance(System.Type)'. The parameter 'wrapperType' of method 'Google.Protobuf.JsonParserTest.Wrappers_Standalone(Type,String,Object)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/Users/jtattermusch/github/protobuf/csharp/src/Google.Protobuf.Test/Google.Protobuf.Test.csproj]
/Users/jtattermusch/github/protobuf/csharp/src/Google.Protobuf.Test/Compatibility/TypeExtensionsTest.cs(105,13): Trim analysis warning IL2067: Google.Protobuf.Compatibility.TypeExtensionsTest.GetMethod_NoSuchMethod(Type,String): 'target' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods', 'DynamicallyAccessedMemberTypes.NonPublicMethods' in call to 'Google.Protobuf.Compatibility.TypeExtensions.GetMethod(Type,String)'. The parameter 'type' of method 'Google.Protobuf.Compatibility.TypeExtensionsTest.GetMethod_NoSuchMethod(Type,String)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/Users/jtattermusch/github/protobuf/csharp/src/Google.Protobuf.Test/Google.Protobuf.Test.csproj]
/Users/jtattermusch/github/protobuf/csharp/src/Google.Protobuf.Test/Compatibility/TypeExtensionsTest.cs(95,13): Trim analysis warning IL2067: Google.Protobuf.Compatibility.TypeExtensionsTest.GetMethod_Success(Type,String): 'target' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods', 'DynamicallyAccessedMemberTypes.NonPublicMethods' in call to 'Google.Protobuf.Compatibility.TypeExtensions.GetMethod(Type,String)'. The parameter 'type' of method 'Google.Protobuf.Compatibility.TypeExtensionsTest.GetMethod_Success(Type,String)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/Users/jtattermusch/github/protobuf/csharp/src/Google.Protobuf.Test/Google.Protobuf.Test.csproj]
/Users/jtattermusch/github/protobuf/csharp/src/Google.Protobuf.Test/Compatibility/TypeExtensionsTest.cs(85,13): Trim analysis warning IL2067: Google.Protobuf.Compatibility.TypeExtensionsTest.GetProperty_NoSuchProperty(Type,String): 'target' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties', 'DynamicallyAccessedMemberTypes.NonPublicProperties' in call to 'Google.Protobuf.Compatibility.TypeExtensions.GetProperty(Type,String)'. The parameter 'type' of method 'Google.Protobuf.Compatibility.TypeExtensionsTest.GetProperty_NoSuchProperty(Type,String)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/Users/jtattermusch/github/protobuf/csharp/src/Google.Protobuf.Test/Google.Protobuf.Test.csproj]
/Users/jtattermusch/github/protobuf/csharp/src/Google.Protobuf.Test/Compatibility/TypeExtensionsTest.cs(75,13): Trim analysis warning IL2067: Google.Protobuf.Compatibility.TypeExtensionsTest.GetProperty_Success(Type,String): 'target' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties', 'DynamicallyAccessedMemberTypes.NonPublicProperties' in call to 'Google.Protobuf.Compatibility.TypeExtensions.GetProperty(Type,String)'. The parameter 'type' of method 'Google.Protobuf.Compatibility.TypeExtensionsTest.GetProperty_Success(Type,String)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/Users/jtattermusch/github/protobuf/csharp/src/Google.Protobuf.Test/Google.Protobuf.Test.csproj]
....
[I removed warnings unrelated to Google.Protobuf assembly]

csharp/src/Google.Protobuf/JsonFormatter.cs

jtattermusch · 2021-11-16T09:56:02Z

csharp/src/Google.Protobuf/Reflection/SingleFieldAccessor.cs

@@ -109,14 +113,48 @@ internal SingleFieldAccessor(PropertyInfo property, FieldDescriptor descriptor)
                // While presence isn't supported, clearing still is; it's just setting to a default value.
                var clrType = property.PropertyType;


Looks like the clrType is now unused?

csharp/src/Google.Protobuf/Reflection/ReflectionUtil.cs

JamesNK · 2021-11-16T17:52:01Z

FTR since I don't know what the "test app" you tested against was, I ran a small experiment myself.

I tested using this app: https://github.com/grpc/grpc-dotnet/tree/dce1ef72d8b88a380136a699a1fb26ca3793a597/testassets/LinkerTestsClient (although I modified it to reference Google.Protobuf source code while testing)

There is a grpc-dotnet unit test that publishes the app with trimming and then executes it: https://github.com/grpc/grpc-dotnet/blob/82dec0ff513059786de5254e62d71c5f55e0ed5b/test/FunctionalTests/Linker/LinkerTests.cs

Actually testing trimming is really difficult because you can't have multiple tests in an individual app because each test will potentially include types and members that otherwise wouldn't be included if tests were run individually.

However, the trimming warnings shouldn't be impacted by this. It analysis all of an assembly, not just the parts that an app uses.

That experiment gave me these warnings:

The warnings you've listed are from the test project.

JamesNK · 2021-11-16T23:57:32Z

nit: Makefile.am needs to be updated

Fixed

jtattermusch

LGTM from the perspective that this enables the use of .NET6 trimming with Google.Protobuf (and removes the warning we currently know about).

Currently there are some limitations of the solution proposed in this PR:

we don't know whether the Google.Protobuf trimming will work realiably with a real life application (we only tried for a small test project)
we don't know how effective the trimming is when used with protobuf messages
we don't have any continuous test that prevents reintroduction of trimming warnings in the future

So for now I'd refrain from claims like "Google.Protobuf supports trimming now" when mentioning this change. I think the current state of things is now more like "initial work to support trimming has been done and Google.Protobuf may work well - but users should test carefully to make sure that there aren't any regressions."

@JamesNK if you are happy with this summary, I can merge once the tests are green.

JamesNK · 2021-11-29T19:57:20Z

we don't know whether the Google.Protobuf trimming will work realiably with a real life application (we only tried for a small test project)

There is a small test project for unit testing, but in real-world apps there are many who are using trimming + gRPC + Google.Protobuf in Blazor.

Whether the JSON functionality works with trimming is still an open question.

So for now I'd refrain from claims like "Google.Protobuf supports trimming now" when mentioning this change. I think the current state of things is now more like "initial work to support trimming has been done and Google.Protobuf may work well - but users should test carefully to make sure that there aren't any regressions."

That's fine. The goal here is aimed at removing warnings.

agocke · 2022-01-13T22:54:46Z

Sorry, I missed the original comments here -- I wanted to note that the static analysis for trimming is meant to be sound, so if no trim warnings are produced, the application should not have any different behavior vs. an untrimmed app at runtime.

Any situation in which the behavior of a trimmed app changes without a warning should be considered a bug on the https://github.com/dotnet/linker tool.

google-cla bot added the cla: yes label Nov 2, 2021

acozzette added c# release notes: yes labels Nov 3, 2021

protobuf-kokoro removed the kokoro:run label Nov 3, 2021

jkotas reviewed Nov 4, 2021

View reviewed changes

csharp/src/Google.Protobuf/Compatibility/TypeExtensions.cs Outdated Show resolved Hide resolved

jkotas reviewed Nov 4, 2021

View reviewed changes

csharp/src/Google.Protobuf/JsonFormatter.cs Outdated Show resolved Hide resolved

acozzette added the kokoro:run label Nov 4, 2021

protobuf-kokoro removed the kokoro:run label Nov 4, 2021

JamesNK mentioned this pull request Nov 5, 2021

Add preliminary trimming support #9183

Closed

JamesNK commented Nov 5, 2021

View reviewed changes

jtattermusch self-assigned this Nov 5, 2021

jtattermusch added the kokoro:run label Nov 9, 2021

protobuf-kokoro removed the kokoro:run label Nov 9, 2021

JamesNK force-pushed the jamesnk/trimming-take3 branch from 1cbf5b9 to 2e9da70 Compare November 12, 2021 18:52

jtattermusch added the kokoro:run label Nov 15, 2021

protobuf-kokoro removed the kokoro:run label Nov 15, 2021

JamesNK force-pushed the jamesnk/trimming-take3 branch from 2e9da70 to 73eb0d7 Compare November 15, 2021 21:44

jtattermusch added the kokoro:run label Nov 16, 2021

protobuf-kokoro removed the kokoro:run label Nov 16, 2021

jtattermusch reviewed Nov 16, 2021

View reviewed changes

acozzette added the kokoro:run label Nov 17, 2021

protobuf-kokoro removed the kokoro:run label Nov 17, 2021

Fix trim warnings

22462b0

JamesNK force-pushed the jamesnk/trimming-take3 branch from 4e63393 to 22462b0 Compare November 17, 2021 01:02

acozzette added the kokoro:run label Nov 17, 2021

jtattermusch added kokoro:force-run and removed kokoro:run labels Nov 29, 2021

protobuf-kokoro removed the kokoro:force-run label Nov 29, 2021

jtattermusch approved these changes Nov 29, 2021

View reviewed changes

jtattermusch merged commit b79ac0e into protocolbuffers:master Nov 30, 2021

perezd mentioned this pull request Dec 13, 2021

continuous builds breaking #9302

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[C#] Fix trim warnings #9182

[C#] Fix trim warnings #9182

JamesNK commented Nov 2, 2021 •

edited

Loading

agocke commented Nov 3, 2021

JamesNK commented Nov 3, 2021

agocke commented Nov 3, 2021

JamesNK commented Nov 4, 2021

agocke commented Nov 4, 2021 •

edited

Loading

JamesNK Nov 5, 2021 •

edited

Loading

JamesNK Nov 5, 2021

JamesNK Nov 5, 2021

jtattermusch commented Nov 9, 2021

JamesNK commented Nov 9, 2021

jtattermusch commented Nov 12, 2021

JamesNK commented Nov 12, 2021

jtattermusch commented Nov 15, 2021

JamesNK commented Nov 15, 2021

jtattermusch commented Nov 16, 2021

jtattermusch left a comment •

edited

Loading

jtattermusch Nov 15, 2021

JamesNK Nov 16, 2021

jtattermusch commented Nov 16, 2021

jtattermusch Nov 16, 2021

JamesNK commented Nov 16, 2021 •

edited

Loading

JamesNK commented Nov 16, 2021

jtattermusch left a comment •

edited

Loading

JamesNK commented Nov 29, 2021

agocke commented Jan 13, 2022

		@@ -109,14 +113,48 @@ internal SingleFieldAccessor(PropertyInfo property, FieldDescriptor descriptor)
		// While presence isn't supported, clearing still is; it's just setting to a default value.
		var clrType = property.PropertyType;

[C#] Fix trim warnings #9182

[C#] Fix trim warnings #9182

Conversation

JamesNK commented Nov 2, 2021 • edited Loading

agocke commented Nov 3, 2021

JamesNK commented Nov 3, 2021

agocke commented Nov 3, 2021

JamesNK commented Nov 4, 2021

agocke commented Nov 4, 2021 • edited Loading

JamesNK Nov 5, 2021 • edited Loading

Choose a reason for hiding this comment

JamesNK Nov 5, 2021

Choose a reason for hiding this comment

JamesNK Nov 5, 2021

Choose a reason for hiding this comment

jtattermusch commented Nov 9, 2021

JamesNK commented Nov 9, 2021

jtattermusch commented Nov 12, 2021

JamesNK commented Nov 12, 2021

jtattermusch commented Nov 15, 2021

JamesNK commented Nov 15, 2021

jtattermusch commented Nov 16, 2021

jtattermusch left a comment • edited Loading

Choose a reason for hiding this comment

jtattermusch Nov 15, 2021

Choose a reason for hiding this comment

JamesNK Nov 16, 2021

Choose a reason for hiding this comment

jtattermusch commented Nov 16, 2021

jtattermusch Nov 16, 2021

Choose a reason for hiding this comment

JamesNK commented Nov 16, 2021 • edited Loading

JamesNK commented Nov 16, 2021

jtattermusch left a comment • edited Loading

Choose a reason for hiding this comment

JamesNK commented Nov 29, 2021

agocke commented Jan 13, 2022

JamesNK commented Nov 2, 2021 •

edited

Loading

agocke commented Nov 4, 2021 •

edited

Loading

JamesNK Nov 5, 2021 •

edited

Loading

jtattermusch left a comment •

edited

Loading

JamesNK commented Nov 16, 2021 •

edited

Loading

jtattermusch left a comment •

edited

Loading