Change XmlFlowAnnotationSource implementation to expose CustomAttributes #1216
Conversation
tlakollo
force-pushed
the
XmlWithCustomAttributes
branch
2 times, most recently
from
87b8b53 to
0a7632d
Compare
| _context.LogMessage (MessageContainer.CreateWarningMessage ($"DynamicallyAccessedMembers attribute was specified but no argument was proportioned", 2020)); | ||
| } else if (attribute.ConstructorArguments.Count == 1) { | ||
| var arguments = attribute.ConstructorArguments.ToArray (); | ||
| return (DynamicallyAccessedMemberTypes) arguments[0].Value; |
There was a problem hiding this comment.
You cannot cast external value to local type
There was a problem hiding this comment.
Same comment as in the previous PR - this is an enum type, this is basically the only case where it's "safe" since it's almost like a primitive type.
There was a problem hiding this comment.
I don't think that's the case. If someone declares it's own type (we matching on names) then you cannot cast the enum type to "runtime" version of the type as they have different assembly identity.
There was a problem hiding this comment.
You're right - we have to go through integer:
return (DynamicallyAccessedMemberTypes)(int)arguments[0].Value;Then it should be correct - as we will simply map the number to the linker understanding of the flags.
- Use ICustomAttributeProvider to simplify when asking for information about annotations - Use of assembly in the attribute is now mandatory - Assembly name now could be * either in the assembly element or in the assembly property inside the attribute element, leaving no assembly name is not supported - Added new error codes, swaped non used codes for new error codes - Changed documentation to reflect the way of use
- Fix review comments
vitek-karas
force-pushed
the
XmlWithCustomAttributes
branch
from
0629231 to
e8e6f54
Compare
|
I rebased to master and fixed any issues after that (mostly warning creation requiring context and location). |
| foreach (ParameterDefinition parameter in method.Parameters) { | ||
| if (paramName == parameter.Name) { | ||
| if (_attributes.ContainsKey (parameter)) | ||
| _context.LogMessage (MessageContainer.CreateWarningMessage (_context, $"There are duplicate parameter names for '{paramName}' inside '{method.Name}' in '{_xmlDocumentLocation}'", 2024, _xmlDocumentLocation)); |
There was a problem hiding this comment.
We found two values for the same thing - we'll pick one of them, but drop the other. I think this should be a warning (since we're warning about other issues in the XML already)
There was a problem hiding this comment.
What if we instead of warning we do something like _attributes[parameter].Add(attributes);
There was a problem hiding this comment.
Sure - we could do that, but I would prefer not to. I don't want to encourage weird specifications. If there would be a good reason for this, then yes, but right now I don't see any. In my mind it's just adding complexity without much value.
| if (attributes.Count () > 0) | ||
| _attributes[method.MethodReturnType] = attributes; | ||
| } else { | ||
| _context.LogMessage (MessageContainer.CreateWarningMessage (_context, $"There is more than one return parameter specified for '{method.Name}' in '{_xmlDocumentLocation}'", 2023, _xmlDocumentLocation)); |
There was a problem hiding this comment.
Same as above - two values for the same thing - we will intentionally drop data in this case so we should warn about it.
…tes (dotnet/linker#1216) - Change XmlFlowAnnotationSource to expose CustomAttributes - Use ICustomAttributeProvider to simplify when asking for information about annotations - Assembly name now could be '*' in the assembly element, there is a new assembly property inside the attribute element, leaving no assembly name in this property means to look for the type in all the loaded assemblies. - Added new error codes, swapped non used codes for new error codes - Changed documentation to reflect the new functionalities - Move attribute sources to the main linker namespace Co-authored-by: Tlakollo <[email protected]> Co-authored-by: vitek-karas <[email protected]> Commit migrated from dotnet/linker@6c9263b
about annotations
assembly property inside the attribute element, leaving no assembly name is not
supported