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

Query Ast returns Empty Result #117

Closed
yangky11 opened this issue Feb 22, 2019 · 13 comments · Fixed by #318
Closed

Query Ast returns Empty Result #117

yangky11 opened this issue Feb 22, 2019 · 13 comments · Fixed by #318

Comments

@yangky11
Copy link

Hello,

I'm using SerAPI to execute Coq code and extract the AST for each Coq command.
I occasionally run into the following error: the returned AST is empty.

(Add () "Defined.")
(Answer 745 Ack)(Answer 745(Added 265"[LOC]"NewTip))(Answer 745 Completed)
(Query ((sid 265)) Ast)
(Answer 746 Ack)(Answer 746(ObjList()))(Answer 746 Completed)

I don't have a simple example to re-produce this, but from my understanding, Query Ast should never return empty result? Any idea?

@ejgallego
Copy link
Owner

What version of SerAPI are you using?

There is actually some bugs in Coq such as coq/coq#6884 so indeed if you cannot provide a test-case I'm afraid we are going to be stuck.

@ejgallego
Copy link
Owner

If you are using 8.8, the first thing is to upgrade to 8.9 as we solved half of the bug there.

@ejgallego
Copy link
Owner

A much more reliable way to get the AST is to use sercomp [in 8.9 too]

@yangky11
Copy link
Author

I was compiling from source using branch v8.9 commit a6b4445.
I'm trying to find a test-case.

@yangky11
Copy link
Author

I cannot find a easy test-case, since the Coq code relies on other code as well. I'll just try to use sercomp instead. Thanks.

@ejgallego
Copy link
Owner

I'd bet that has to do with people using "nested proofs" or weird effectul commands inside proofs.

I can provide you a workaround tho if you want to keep using the interactive protocol, which is to expose the Parse call that sercomp uses. This will be 100% reliable.

Would that help?

@yangky11
Copy link
Author

I'll have a try. Thanks!

ejgallego added a commit that referenced this issue Feb 22, 2019
In some circumstances parsing may be better than calling the STM.

Usage:
```
(Parse ((ontop sid)) "text")
```
where `ontop` is optional.

**Note:** In Coq 8.9 there is a strong limitation where parsing can
only be performed at the "tip" of the document, that is to say, just
after an add.

See #117, cc: @yangky11
@ejgallego
Copy link
Owner

Ok, see 337f9b6

The command is very limited [see commit message] but it may help in your case.

@ejgallego ejgallego added this to the 0.6.1 milestone Feb 22, 2019
@yangky11
Copy link
Author

yangky11 commented Feb 22, 2019

It works pretty well. What is the text in (Parse ((ontop sid)) "text")? Is it constrained to be exactly the same as the command added before?

@ejgallego
Copy link
Owner

ejgallego commented Feb 22, 2019

It is the input to the parser, you can indeed pass any valid Coq input, it doesn't have to be the same.

Note however the limitation with ontop, it has to be indeed the previous sid (or you have to omit it)

This will be lifted in the future, maybe it even works on 8.10 already.

@yangky11
Copy link
Author

Got it, thanks!

@ejgallego ejgallego modified the milestones: 0.6.1, 0.7.0 Mar 27, 2019
ejgallego added a commit to ejgallego/opam-repository that referenced this issue Apr 23, 2019
CHANGES:

* [serapi ] Add `Parse` command to parse a sentence; c.f.
             ejgallego/coq-serapi#117
             (@ejgallego) (cc: @yangky11)
 * [sercomp] Add "print" `--mode` to print the input Coq document
             (@ejgallego) (cc: @Ptival)
 * [serlib ] Serialize `Universe.t` (@ejgallego, request by @yangky11)
 * [sercomp] Merge `sercomp` and `compser`, add `--input` parameter to `sercomp`
             (@palmskog) (cc: @ejgallego)
 * [serlib ] Much improved support for serialization of `Environ.env`
             (@yangky11 and @ejgallego c.f. ejgallego/coq-serapi#118)
 * [serapi ] Make sure every command ends with `Completed`, even if it produced
             an exception (@brando90 and @ejgallego c.f. ejgallego/coq-serapi#124)
 * [sercomp] Add `--mode=kexp` to output the final kernel environment.
             (@ejgallego c.f. ejgallego/coq-serapi#119)
 * [serlib ] Serialize more internal environment fields (@ejgallego c.f. ejgallego/coq-serapi#119)
 * [serlib ] Improvements in serialization org (@ejgallego)
 * [serlib ] Serialize kernel entries (@ejgallego @palmskog)
 * [serlib ] Fix critical bug on `Constr` deserialization; reported by @palmskog,
             fix by @SkySkimmer.
 * [sertop]  Fix backtrace printing when using `--debug` (@ejgallego)
 * [serlib ] Don't serialize VM values (@ejgallego, bug report by @palmskog)
 * [serapi ] Output location on tokenization (@ejgallego , idea by @palmskog)
 * [serapi ] Add basic documentation of the protocol (@ejgallego cc ejgallego/coq-serapi#109)
@ejgallego ejgallego modified the milestones: 0.7.0, 0.8.0, upstream Oct 25, 2019
@tom-p-reichel
Copy link

tom-p-reichel commented Jun 14, 2022

It's been 3 years. Is Parse still a best practice on version 8.15.0+0.15.0?

I also have a minimal example that seems to reproduce this issue on the latest version. It indeed involves nested proofs, as was predicted in this thread.

(Add () "\n\nSet Nested Proofs Allowed.\n\nTheorem outer : nat.\n pose 3.\nTheorem inner : nat.\n intros.\n apply 1.\nQed.\n apply inner.\nQed.")
(Query ((sid 9)) Ast)

@ejgallego
Copy link
Owner

Parse is indeed still a workaround.

The problem you point out still exists, unfortunately there is not a lot we can do in SerAPI as this is a bug in Coq itself.

A fix is planned upstream, but no idea when it will arrive.

ejgallego added a commit that referenced this issue Feb 14, 2023
Thanks to everyone that helped and used this project for almost 7
years!

The following issues are solved by coq-lsp:

closes #252, closes #261, closes #234, closes #202, closes #117,
closes #49, closes #32, closes #26, closes #24, closes #13
ejgallego added a commit that referenced this issue Feb 14, 2023
Thanks to everyone that helped and used this project for almost 7
years!

The following issues are solved by coq-lsp:

closes #252, closes #261, closes #234, closes #202, closes #117,
closes #49, closes #32, closes #26, closes #24, closes #21,
closes #18, closes #13
ejgallego added a commit that referenced this issue Feb 14, 2023
Thanks to everyone that helped and used this project for almost 7
years!

The following issues are solved by coq-lsp:

closes #252, closes #261, closes #234, closes #202, closes #117,
closes #49, closes #32, closes #26, closes #24, closes #21,
closes #18, closes #13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants