Over-design for `BackendManager`?

Question

Over-design for `BackendManager`?

Opened this issue a year ago · 3 comments

Description

claripy.backends is originally a submodule of claripy, but in claripy/__init__.py, backends is reassigned by _backend_manager.BackendManager() and used to register other backend.

from . import backend_manager as _backend_manager
backends = _backend_manager.BackendManager()
backends._register_backend(_backends_module.BackendConcrete(), "concrete", True, True)
backends._register_backend(_backends_module.BackendVSA(), "vsa", False, False)

I know that it's for the flexibility to add backends by users themselves, but this name collision between backends:Module and backends:BackendManager really causes some problems in type infering. The language server typically mark it as module other than BackendManager.

There is a simple fix without BackendManager:

remove all BackendManager
move class Backend from claripy/backends/__init__.py to claripy/backends/backend.py( avoid recursive import)
In claripy/backends/__init__.py, use from .backend_z3 import BackendZ3 to import correct class
In claripy/backends/__init__.py, use z3 = BackendZ3 , __all__ = ["z3"] to create alias

Alternatives

No response

Additional context

No response

Answer 1 · 2023-05-15T19:15:24.000Z

I agree that it's over-engineered. Would you be willing to send a PR with this plan executed?

…

On Mon, May 15, 2023 at 9:02 AM syheliel ***@***.***> wrote: Description claripy.backends is originally a submodule of claripy, but in claripy/__init__.py, backends is reassigned by _backend_manager.BackendManager() and used to register other backend. from . import backend_manager as _backend_manager backends = _backend_manager.BackendManager() backends._register_backend(_backends_module.BackendConcrete(), "concrete", True, True) backends._register_backend(_backends_module.BackendVSA(), "vsa", False, False) I know that it's for the flexibility to add backends by users themselves, but this name collision between *backends:Module* and *backends:BackendManager* really causes some problems in type infering. The language server typically mark it as module other than BackendManager. [image: image] <https://urldefense.com/v3/__https://user-images.githubusercontent.com/45957390/238404102-31f2d87a-2915-44de-9207-3fe28cd310af.png__;!!IKRxdwAv5BmarQ!afXT7Vr6FF_RCH2fkgXFpPNaQlqYxePFIHVP4uu5f7b4NF5xvATa7zWmQ6oPJrkeyly8W7npwWbShQEf9BzR$> There is a simple fix without BackendManager: 1. remove all BackendManager 2. move class Backend from claripy/backends/__init__.py to claripy/backends/backend.py( avoid recursive import) 3. In claripy/backends/__init__.py, use from .backend_z3 import BackendZ3 to import correct class 4. In claripy/backends/__init__.py, use z3 = BackendZ3 , __all__ = ["z3"] to create alias Alternatives *No response* Additional context *No response* — Reply to this email directly, view it on GitHub <https://urldefense.com/v3/__https://github.com/angr/claripy/issues/353__;!!IKRxdwAv5BmarQ!afXT7Vr6FF_RCH2fkgXFpPNaQlqYxePFIHVP4uu5f7b4NF5xvATa7zWmQ6oPJrkeyly8W7npwWbShYH5cUPo$>, or unsubscribe <https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AA2LHF62WWH37VEMRVNIRHTXGJHPNANCNFSM6AAAAAAYCM6JGU__;!!IKRxdwAv5BmarQ!afXT7Vr6FF_RCH2fkgXFpPNaQlqYxePFIHVP4uu5f7b4NF5xvATa7zWmQ6oPJrkeyly8W7npwWbShQ_pQqUo$> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

Answer 2 · 2023-05-16T07:23:06.000Z

I'm planning to add it. Since the modification overlaps with some part in #352 , so I will do this PR when #352 is merged.

Answer 3 · 2023-05-16T19:35:22.000Z

@syheliel I would advise you do it in a separate parallel PR instead of waiting for that one to be merged, as this is a much smaller change that can be reviewed and merged more quickly.