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

Improve Importer TryToUseMethodDefs and TryToUseFieldDefs options 2 #443

Merged
merged 2 commits into from
Dec 30, 2021

Conversation

wwh1004
Copy link
Contributor

@wwh1004 wwh1004 commented Dec 29, 2021

Obfuscator may rename many methods/fields into same name then TryResolveMethod/TryResolveField will return inconsistent method/field.

Previous PR contains some problems and I fixed it. In previous PR you mentioned 'privatescope' visibility. I check some documents and find it is visible in the same module and can't be referenced in MemberRef table. I think these code won't get error when encountering 'privatescope' visibility. Should it be handled specially?

…lveMethod/TryResolveField will return inconsistent method/field.
@CreateAndInject
Copy link
Contributor

CreateAndInject commented Dec 29, 2021

It's better to use AssemblyNameComparer or SigComparer to determine if is the same Module/Assembly

src/DotNet/Importer.cs Outdated Show resolved Hide resolved
@wtfsck
Copy link
Contributor

wtfsck commented Dec 29, 2021

Previous PR contains some problems and I fixed it. In previous PR you mentioned 'privatescope' visibility. I check some documents and find it is visible in the same module and can't be referenced in MemberRef table. I think these code won't get error when encountering 'privatescope' visibility. Should it be handled specially?

I don't think it's necessary to check for privatescope here.

@wtfsck
Copy link
Contributor

wtfsck commented Dec 29, 2021

It's better to use AssemblyNameComparer or SigComparer to determine if is the same Module/Assembly

True, but in this case only the name is compared and there's no public API that only compares the name.

@wtfsck wtfsck merged commit 63a06e6 into 0xd4d:master Dec 30, 2021
@wtfsck
Copy link
Contributor

wtfsck commented Dec 30, 2021

Thanks, merged!

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.

3 participants