Skip to content

Handle empty string parameter value in MetadataLoadContext - #61457

Merged
buyaa-n merged 2 commits into
dotnet:mainfrom
svick:mlc-empty-string-param
Nov 24, 2021
Merged

Handle empty string parameter value in MetadataLoadContext#61457
buyaa-n merged 2 commits into
dotnet:mainfrom
svick:mlc-empty-string-param

Conversation

@svick

Copy link
Copy Markdown
Member

Fixes#57238.

Also add test for null parameter default value, since that is another special case.

@ghostghost added area-System.Reflection.Metadata community-contribution Indicates that the PR has been added by a community member labels Nov 11, 2021
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @buyaa-n
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #57238.

Also add test for null parameter default value, since that is another special case.

Author:svick
Assignees:-
Labels:

area-System.Reflection.Metadata

Milestone:-

public void Foo4(short s = -34) { }
public void Foo5(decimal d = 1234m) { }
public void Foo6([DateTimeConstant(ticks: 8736726782)] DateTime dt) { }
public void Foo7(string s1 = "foo", string s2 = "") { }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NIT: maybe add a test case for null default?

Suggested change
publicvoidFoo7(strings1="foo",strings2=""){}
publicvoidFoo7(strings1="foo",strings2="",strings3=null){}

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Sure, done.

@buyaa-nbuyaa-n left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Left and NIT, overall LGTM, thanks!

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Reflection.Metadatacommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ParameterInfo.RawDefaultValue throws when default value is string.empty

4 participants

@svick@steveharter@buyaa-n@joperezr