Skip to content

Do not output attribute information when unparsing TermLike #3823

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

Merged
merged 8 commits into from
Apr 24, 2024

Conversation

geo2a
Copy link
Contributor

@geo2a geo2a commented Apr 22, 2024

When pretty-printing a TermLike, the term attributes are serialized into text and in-lined into the unparsed Kore term, leading to results like this one:

    Could not infer the equation requirement:
        /* Spa */
        \equals{SortBool{}, _}(
            /* T Fn D Spa */
            Lbl'Unds-LT-'Int'Unds'{}(
                /* T Fn D Spa */
                Lblf'LParUndsRParUnds'TEST'Unds'Int'Unds'Int{}(
                    /* T Fn D Sfa */ ConfigVarI:SortInt{}
                ),
                /* T Fn D Sfa Cl */ \dv{SortInt{}}("0")
            ),
            /* T Fn D Sfa Cl */ \dv{SortBool{}}("true")
        )

I'd argue that we do not care as much about the exact attributes of TermLikes anymore today. Therefore I propose to remove the attributes, leading to a much cleaner log output:

    Could not infer the equation requirement:
        \equals{SortBool{}, _}(
            Lbl'Unds-LT-'Int'Unds'{}(
                Lblf'LParUndsRParUnds'TEST'Unds'Int'Unds'Int{}(
                    ConfigVarI:SortInt{}
                ),
                \dv{SortInt{}}("0")
            ),
            \dv{SortBool{}}("true")
        )

The integration tests fail, but I will fix them once we have decided if we want this merged.

@geo2a geo2a marked this pull request as ready for review April 22, 2024 07:48
@geo2a geo2a marked this pull request as draft April 22, 2024 08:37
@geo2a geo2a marked this pull request as ready for review April 22, 2024 09:48
Copy link
Contributor

@goodlyrottenapple goodlyrottenapple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, tho you will have to likely update a lot of integration golden files

@jberthold
Copy link
Member

Looks good, tho you will have to likely update a lot of integration golden files

➕ Yes. As suggested on slack:
13 hours ago
Maybe sed -i -e 's,/\*[^*]*\*/,,g' -e '/^ *$/d' <EXPECTED_OUTPUT_FILES> in the integration tests?

@geo2a geo2a force-pushed the georgy/unclutter-termlike-unparser branch from ad53102 to 783a9e9 Compare April 23, 2024 14:22
@geo2a geo2a force-pushed the georgy/unclutter-termlike-unparser branch from 783a9e9 to 50e0601 Compare April 23, 2024 14:25
@rv-jenkins rv-jenkins merged commit 0fa22f7 into master Apr 24, 2024
@rv-jenkins rv-jenkins deleted the georgy/unclutter-termlike-unparser branch April 24, 2024 14:26
@geo2a geo2a mentioned this pull request Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants