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

[cpp] breakup generator #11785

Open
wants to merge 14 commits into
base: development
Choose a base branch
from

Conversation

Aidan63
Copy link
Contributor

@Aidan63 Aidan63 commented Oct 7, 2024

I frequently find myself getting into a bit of a muddle navigating around gencpp, so here's an attempt at making it a bit more managable by splitting it into several modules.

image

Disclaimer, before I started bodging gencpp a few years ago I had zero ocaml knowledge, I still feel like I have no real idea of what makes good / manageable ocaml, so thoughts welcome!

Anyway, heres some explanation as to how and why I split things up.

  • CppTypeUtils: Various helper functions for t, tclass, tvar, and a few others. No dependencies on anything else.
  • CppExprUtils: Various helper functions for texpr. No dependencies.
  • CppStrings: Hashing, escaping, and other string stuff, No dependencies.
  • CppAst & CppAstTools: Contains the cpp generators ast types and functions for basic checks, printing, etc. Opens CppTypeUtils.
  • CppRetyper: Functions for converting t and texrp to cpp ast types. Depends on all the above.
  • Modules in the gen/ folder should mostly be obvious based on their name, they're responsible for generating C++ from the cpp ast types. CppGen contains a bunch of functions which are used by the other modules and CppReference is used to determine what needs to be forward declared or included. Depends on all the above.

That now leaves gencpp fairly slim and just sitching a few calls to the gen module. Generating header and implementation files is now completely independent whereas before headers had to be generated after implementation files due to depending on state generated by the implementation generation code.

@Simn
Copy link
Member

Simn commented Oct 8, 2024

I had the same problem with the old matcher.ml, and then after splitting it up I found it even more difficult to find anything... But in principle I definitely agree with doing something like this.

@hughsando Please let me know what you think!

@Aidan63
Copy link
Contributor Author

Aidan63 commented Oct 8, 2024

Yeah, thats the key thing I still haven't worked out. Is this actually more managable, or have I just spread things over multiple files!

@hughsando
Copy link
Member

I am happy in principle, and what you propose seems reasonable.
I have not objections.

@Simn
Copy link
Member

Simn commented Oct 8, 2024

The only thing is that I seem to remember some arcane OCaml (editor?) issue with capitalized file names, but there's a good chance that this is no longer relevant.

@Aidan63
Copy link
Contributor Author

Aidan63 commented Oct 8, 2024

Don't think I ever noticed that files were all lower case before... easy enough to change.

Also, is there any standard formatting? gencpp seems to be indented to 3 spaces but other files are 4. I put all these new files through ocamlformat and it idented to two spaces...

@Simn
Copy link
Member

Simn commented Oct 8, 2024

Yeah gencpp had Australian formatting.

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