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

Use proper names for datatypes, constructors, and recursors in saw-core term AST #1285

Closed
brianhuffman opened this issue May 4, 2021 · 4 comments
Assignees
Labels
tech debt Issues that document or involve technical debt

Comments

@brianhuffman
Copy link
Contributor

The saw-core AST currently uses the NameInfo type to represent structured names for defined constants, primitives, and free variables (ExtCns). These are registered in a SAWNamingEnv, which can be used to determine the shortest unambiguous version of each NameInfo during pretty printing.

However, the names of datatypes (DataTypeApp), data constructors (CtorApp), and recursors (RecursorApp) are represented with only an Ident instead of a full NameInfo. Among other problems, this means that pretty printing of these names cannot use the shortest unambiguous version; they have to be printed always with a fully qualified name.

We should change the representation of these term AST constructors so that each name comes with a NameInfo and a unique VarIndex. To this end, it might be useful to introduce a new type (probably called just Name) in Verifier/SAW/Name.hs that pairs a NameInfo with a VarIndex; then we can replace Ident with Name in all those places.

@brianhuffman brianhuffman added the tech debt Issues that document or involve technical debt label May 4, 2021
@robdockins
Copy link
Contributor

This impacts some stuff I'm working on, so I'll take this on.

@robdockins robdockins self-assigned this May 13, 2021
@brianhuffman
Copy link
Contributor Author

Commit 02f2be2, which replaced the GlobalDef constructor (which used Ident) with Primitive (which uses ExtCns), will probably be relevant. This was part of the PR GaloisInc/saw-core#163.

@robdockins
Copy link
Contributor

An annoying aspect of this is that the Nat datatype is deeply built-in. A variety of internal operations (conversions and matchers and such) want to recognize that datatype and its constructors in a context where the SharedContext and its naming environment aren't available.

@robdockins
Copy link
Contributor

I believe this is fixed via #1310

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech debt Issues that document or involve technical debt
Projects
None yet
Development

No branches or pull requests

2 participants