Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bugs in DAC names that either contain non-closed generics or non-assembly qualified names #1172

Merged
merged 7 commits into from
Aug 22, 2023

Conversation

ryanmolden
Copy link
Member

@ryanmolden ryanmolden commented Jul 26, 2023

Fix DAC name parser to deal with non-closed generic types and non-assembly qualified names (see bug #897).

Add lots of comments to parser as its complex due to having to deal with tragedies like the type name in the ParseGenericWithComplexGenericArgs2 test.

Replace uses of char literals in numerous places with their named constants I was using elsewhere.

Correct numerous comment spelling errors (thanks VS spell checker).

Add numerous tests both for the case in 897, some variants of it, and another case I encountered internally.

Add support for non-traditional generics as produced by FSharp and various obfuscators.

@ryanmolden ryanmolden changed the title Fix bugs in DAC names that either contain non-closed generics or non-… Fix bugs in DAC names that either contain non-closed generics or non-assembly qualified names Jul 26, 2023
@mikem8361 mikem8361 requested a review from leculver July 28, 2023 17:37
@leculver
Copy link
Contributor

Sorry I have been on leave and will dig into this PR this week. Thank you for taking a look here!

ryanmolden and others added 7 commits August 22, 2023 08:58
…assembly qualified names (see bug microsoft#897)

Add lots of comments to parser as its complex due to having to deal with tragedies like the type name in ParseGenericWithComplexGenericArgs2

Replace uses of char literals in numerous places with their named constants I was using elsewhere

Correct numerous comment spelling errors (thanks VS spell checker)

Add numerous tests both for the case in 897, some variants of it, and another case I encountered internally
@leculver leculver force-pushed the dev/rmolden/FixDACNameParserBug branch from d722431 to 712c196 Compare August 22, 2023 15:59
Copy link
Contributor

@leculver leculver left a comment

Choose a reason for hiding this comment

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

LGTM, I made a minor fix to reduce allocations.
This worked well on the test dumps I threw at it, and passes all tests.

@leculver leculver merged commit d6684cc into microsoft:main Aug 22, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants