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

Properly parse qualified identifiers #3111

Merged
merged 28 commits into from Aug 6, 2019
Merged

Conversation

ajreynol
Copy link
Member

This refactors the smt2 parser so that it properly parses qualified identifiers (the generic form of function application heads in smt2). This required refactoring of the termNonVariable grammar in Smt2.g.

Specifically:

  • This PR merges the cases of termNonVariable of (1) a type-ascripted term with head as, (2) function variables applied to argument list (3) indexed functions applied to argument list, and (4) array constants, which can be seen as (as const (Array T1 T2)) applied to an argument.
    These are now all cases of a qualified identifier (qualIdentifier) applied to argument list, which follows terms in section 3.6 of http://smtlib.cs.uiowa.edu/papers/smt-lib-reference-v2.6-r2017-07-18.pdf.
  • Merges indexed function names and symbols into a identifier, following the definition of identifier in section 3.3 of http://smtlib.cs.uiowa.edu/papers/smt-lib-reference-v2.6-r2017-07-18.pdf.
  • A few checks had to be rearranged to preserve the old behavior, including moving the hack for unary minus in sygus version 1.0.

This fixes #2832.

Copy link
Member

@mpreiner mpreiner left a comment

Choose a reason for hiding this comment

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

LGTM, one minor suggestion.

src/parser/smt2/Smt2.g Outdated Show resolved Hide resolved
@4tXJ7f
Copy link
Member

4tXJ7f commented Jul 24, 2019

I didn't have time to look at this in detail yet. One high-level comment that I have: We have decided to move logic out of the parser files as much as possible (to make it easier to move away from ANTLR3) but this PR adds quite a bit of logic in the parser file itself. Is there any chance that we could move this code to the parser class?

@ajreynol
Copy link
Member Author

I agree we should move code to the parser files. This PR actually doesn't add much code, it just rearranges it. Perhaps we should do this on a followup PR?

ajreynol and others added 3 commits July 24, 2019 09:44
Co-Authored-By: Mathias Preiner <mathias.preiner@gmail.com>
@mpreiner
Copy link
Member

Yes, I think it's a good idea to move the logic to the parser files in a follow-up PR.

@ajreynol
Copy link
Member Author

Ready to merge?

@mpreiner
Copy link
Member

I think we should still wait to get a second review for that. @4tXJ7f do you have any time soon to do a quick review?

@4tXJ7f
Copy link
Member

4tXJ7f commented Aug 1, 2019

I'll try to do it tomorrow evening (Thursday).

src/parser/smt2/Smt2.g Outdated Show resolved Hide resolved
src/parser/smt2/Smt2.g Outdated Show resolved Hide resolved
ajreynol and others added 4 commits August 6, 2019 10:02
Co-Authored-By: Aina Niemetz <aina.niemetz@gmail.com>
Co-Authored-By: Aina Niemetz <aina.niemetz@gmail.com>
@aniemetz aniemetz merged commit f06ea7c into cvc5:master Aug 6, 2019
@ajreynol ajreynol deleted the smt2QualId branch August 6, 2019 22:19
@4tXJ7f 4tXJ7f added this to Review in progress in Parser Migration via automation Feb 10, 2020
@ajreynol ajreynol moved this from Review in progress to Done in Parser Migration Feb 27, 2020
@mpreiner mpreiner removed this from Done in Parser Migration May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complex Complexity normal Priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

datatypes: type checking issue
4 participants