Fix resolving from a ModuleReference #730

Merged

AnakinRaW merged 2 commits from resolve-multi-module into master

2021-04-23 16:46:14 -05:00

AnakinRaW commented

2021-03-15 06:30:50 -05:00

(Migrated from github.com)

As requested here is the failing PR for #725 .
I managed to use the existing test resources mma.exe to reproduce the error.

The test simple asserts that no exception shall get thrown, but since I'm not sure how you want to handle this situation, this is just a placeholder and should be replaced once a decision on actual behavior is made.

As requested here is the failing PR for #725 . I managed to use the existing test resources `mma.exe` to reproduce the error. The test simple asserts that no exception shall get thrown, but since I'm not sure how you want to handle this situation, this is just a placeholder and should be replaced once a decision on actual behavior is made.

jbevain commented

2021-03-15 18:44:35 -05:00

(Migrated from github.com)

That's related to #652. Funny that the code assumes this works.

jbevain commented

2021-03-15 19:41:11 -05:00

(Migrated from github.com)

Also fix #652

AnakinRaW commented

2021-03-16 08:41:01 -05:00

(Migrated from github.com)

changes look good for me, thanks.
Maybe a few other thoughts:

I guess it should be NOT part of Cecil to find a weak "ModuleReference match". But since MetadataResolver::Resolve is virtual i guess there should be possibility for anyone to have custom logic. I like that!
At this point I think it would be cool to have a nullable syntax/doc for properties. Are there any concerns about this? Is this a new issue? Enabling C# nullable syntax might reveal more such issues.

changes look good for me, thanks. Maybe a few other thoughts: 1. I guess it should be NOT part of Cecil to find a weak "ModuleReference match". But since MetadataResolver::Resolve is `virtual` i guess there should be possibility for anyone to have custom logic. I like that! 2. At this point I think it would be cool to have a nullable syntax/doc for properties. Are there any concerns about this? Is this a new issue? Enabling C# nullable syntax might reveal more such issues.