Skip to content

[ new ] Support arbitrary Name in IBindVar #3526

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

spcfox
Copy link
Contributor

@spcfox spcfox commented Mar 26, 2025

Description

Replaces String with Name in the IBindVar signature. This makes IBindVar more consistent and allows the binding of machine-generated names. Using machine-generated names in IBindVar can be useful in compiler and elaborator scripts.

Should this change go in the CHANGELOG?

  • If this is a fix, user-facing change, a compiler change, or a new paper
    implementation, I have updated CHANGELOG_NEXT.md (and potentially also
    CONTRIBUTORS.md).

@spcfox
Copy link
Contributor Author

spcfox commented Mar 26, 2025

This PR is ready to review. The fixes in LSP-lib, elab-util, frex and idrall (for katla) should be enough for a green CI

@spcfox
Copy link
Contributor Author

spcfox commented Mar 26, 2025

The status of the pack collection with this branch:

[ warning ] The following packages failed to build:
            - barbies
            - bounded-doubles-hedgehog-generators(failing dependencies: pretty-show, sop)
            - chem-generators(failing dependencies: pretty-show, sop)
            - cyby-draw(failing dependencies: json-simple)
            - dependent-map(failing dependencies: pretty-show, sop)
            - deptycheck
            - dom-mvc(failing dependencies: json-simple)
            - dom-mvc-extra(failing dependencies: json-simple, barbies)
            - hedgehog(failing dependencies: pretty-show, sop)
            - http(failing dependencies: sop)
            - json
            - json-simple
            - lana(failing dependencies: json, json-simple)
            - node
            - parser-tsv
            - parser-webidl(failing dependencies: sop)
            - pretty-show
            - refined-json(failing dependencies: json-simple)
            - refined-tsv(failing dependencies: parser-tsv)
            - sop
            - sqlite3
            - sqlite3-rio(failing dependencies: sqlite3)
            - stellar-http(failing dependencies: node)
            - stellar-sql(failing dependencies: sqlite3, json)
            - tls(failing dependencies: sop)
            - tyttp-adapter-node(failing dependencies: node)
            - tyttp-json(failing dependencies: json, sop, node)
            - webidl(failing dependencies: sop)
[ warning ] 28 packages failed to build.

@stefan-hoeck
Copy link
Contributor

Looking at the failing pack libraries, this will require quite a bit of work till the pack collection is up and running again. Many of the failing packages are my own, so they should be fine. deptycheck and node are not, so we probably should get in touch with their maintainers.

I for my part am ready to go with this and merge the adjustments made to elab-util as soon as reviewers are ready to merge this.

Thanks, spcfox, for going through the trouble and also checking the pack collection!

@spcfox
Copy link
Contributor Author

spcfox commented Mar 27, 2025

I'm working with @buzden, we're ready to make changes to deptycheck

@buzden
Copy link
Collaborator

buzden commented Mar 27, 2025

As a comment beside libraries changes, I've talked about this change with Edwin in 2021 and he agreed that this should be done and said that he didn't remember why the current design of IBindVar was chosen.

@dunhamsteve
Copy link
Collaborator

I believe we need to bump the TTC version in src/Core/Binary.idr. Otherwise, it will fail with "file: Corrupt TTC data for Name" when it reads .ttc files made by the current Idris.

@spcfox
Copy link
Contributor Author

spcfox commented Mar 27, 2025

I'm not sure if this kind of code is supposed to work. It doesn't right now

definition : Decl
definition =
   IDef EmptyFC `{ foo }
        [ PatClause EmptyFC `( foo ~(IBindVar EmptyFC $ UN $ Basic "x") )
                             (IVar EmptyFC (DN "y" $ UN $ Basic "x"))]

@buzden
Copy link
Collaborator

buzden commented Apr 24, 2025

I'm not sure if this kind of code is supposed to work. It doesn't right now

definition : Decl
definition =
   IDef EmptyFC `{ foo }
        [ PatClause EmptyFC `( foo ~(IBindVar EmptyFC $ UN $ Basic "x") )
                             (IVar EmptyFC (DN "y" $ UN $ Basic "x"))]

I think it should, because I understand DN as a purely pretty-printing thing, which should not influence on typechecking

@spcfox
Copy link
Contributor Author

spcfox commented Apr 28, 2025

I'm not sure if this kind of code is supposed to work. It doesn't right now

definition : Decl
definition =
   IDef EmptyFC `{ foo }
        [ PatClause EmptyFC `( foo ~(IBindVar EmptyFC $ UN $ Basic "x") )
                             (IVar EmptyFC (DN "y" $ UN $ Basic "x"))]

Ok, this is not related to the current PR, created an issue #3542 on this behavior

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.

4 participants