search

jbevain / cecil

Cecil is a library to inspect, modify and create .NET programs and libraries.
MIT License
2.72k stars 620 forks source link

Make Cecil guess a type of constant when it cannot resolve a reference assembly #880

Closed tanakorn closed 1 year ago

tanakorn commented 1 year ago

Make Cecil guess a type of constant when it cannot resolve a reference assembly. Scenario: We are writing a program that reads an assembly, edit the loaded assembly, and then writes it back to file. When the program is working with an assembly A.dll that references to B.dll but B.dll is missing when the program is working. Sometimes, the program cannot write the assembly back and it happens when A.dll uses an enum defined in B.dll (which is not present).

This PR make Cecil does the best effort to guess the type of enum and not fails in the specified scenario.

SimonCropp commented 1 year ago

in what scenario would you have A refs B, but B is not present? a corrupted install?

ltrzesniewski commented 1 year ago

I don't think guesswork is in the spirit of Cecil, which aims to accurately round-trip assemblies. The underlying type of an enum is not necessarily an int.

tanakorn commented 1 year ago

You can think of it as a corrupted installation. B.dll is not supposed to be missing, but in our environment, it is not surprising if that happens.

And we found that in .NET framework, even B.dll is not present when A.dll is executed, but if A.dll does not invoke any methods in B.dll, it still works.

That is why we do not want to fail our tool, and we made this change to sidestep the problem. It has been working well in our setup, and we think that the community might be interested so we would like to contribute back.

Please let me know if you think this will be beneficial to Mono.Cecil in general.

SimonCropp commented 1 year ago

IMO this is not something Cecil should be handling

tanakorn commented 1 year ago

Okay, if you think Cecil should not handle this, I will close the PR.

SimonCropp commented 1 year ago

just to be clear @ltrzesniewski and I are only tourists. @jbevain has the final say

tanakorn commented 1 year ago

I see. Thanks for the information. This is my first time creating pull request so not quite sure what is the protocol. @jbevain Please let me know what you think about the PR. If you think it could be useful, I will re-create it.

jbevain commented 1 year ago

Thanks for taking the time to create a PR @tanakorn.

However I agree with @SimonCropp and @ltrzesniewski here. Cecil shouldn't try to guess which type is an enum. Thanks!