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

Simplify option parsing code in pyk #4484

Open
dwightguth opened this issue Jun 26, 2024 · 1 comment
Open

Simplify option parsing code in pyk #4484

dwightguth opened this issue Jun 26, 2024 · 1 comment

Comments

@dwightguth
Copy link
Collaborator

I had the recent experience of adding a new command line option to pyk kompile recently and noticed that it's a good bit more complicated than the process of adding a command line option to the Java codebase.

Here is a side by side comparison:

Java:

  • Declaratively add option and its description and default
  • Use option

Python:

  • update exec_kompile in __main__.py
  • add field to KompileOptions class
  • add default value to KompileOptions class
  • Declaratively add option and its description
  • add field to LLVMKompile class
  • add parameter to LLVMKompile constructor
  • add initializer to body of LLVMKompile constructor
  • propagate flag from LLVMKompile to JVM

In general I've noticed a lot of code in pyk that simply propagates options back and forth between different layers of abstraction. This adds a good bit of flexibility, because you can always easily control which flags are set to which values at each level. But it makes there be a large amount of boilerplate code like this, which is potentially prone to bugs if a site where an option needs to be propagated is missed. I'm not entirely sure what a scriptable but less-full-of-boilerplate solution might look like, but you might want to consider thinking about it. As the code base grows, you may find that this is a source of fragility in the overall stability of the tool.

@ehildenb
Copy link
Member

Related: #4188
Related: runtimeverification/pyk#885
Related: runtimeverification/pyk#906

We have prioritized re-usability of the options downstream, which indeed causes a decent amount of boilerplate. We've also prioritized supporting both CLI and TOML file configuration of options in Kontrol, but having one source of truth on the "defaults". Unfortunately, it hasn't quite all stitched together yet.

I think we can definitely do better here, the question is whether it's worth investing in now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants