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

Extraction inside a module #10739

Closed
Lysxia opened this issue Sep 7, 2019 · 12 comments · Fixed by #17344
Closed

Extraction inside a module #10739

Lysxia opened this issue Sep 7, 2019 · 12 comments · Fixed by #17344
Labels
kind: anomaly An uncaught exception has been raised. part: extraction The extraction mechanism.
Projects
Milestone

Comments

@Lysxia
Copy link
Contributor

Lysxia commented Sep 7, 2019

Description of the problem

The following snippet, with a definition at the toplevel, then a Recursive Extraction command inside a module, extracts to literally nothing:

Require Extraction.

Definition f := 0.
Module M.
  Recursive Extraction f.
  (* No extracted output at all, not even "f" *)

  Extraction f.
  (* Error: Anomaly "reference not found in extracted structure." *)

Related to #9365 which was redirected to QuickChick, but I've minimized the example to that, which is no longer within QuickChick's jurisdiction.

Coq Version

tested on 8.9 and 8.10+alpha (Aug 18 2019)

@maximedenes
Copy link
Member

Wow, thanks for the reduced test case! Maybe @pi8027 could be interested in this.

@pi8027 pi8027 added the part: extraction The extraction mechanism. label Sep 8, 2019
@pi8027
Copy link
Contributor

pi8027 commented Sep 8, 2019

The Extraction f command does the same thing as Recursive Extraction f, find f in the result, then print it. So the second problem would be disappeared if Recursive Extraction gets fixed.

@ppedrot
Copy link
Member

ppedrot commented Sep 8, 2019

Not that I want to play Cassandra, but asking to be able to extract definitions in the middle of a module does look like giving a stick to get beaten with...

@Lysxia
Copy link
Contributor Author

Lysxia commented Sep 8, 2019

I'm actually not asking for that feature. I think it would be okay to disallow it, and display a clear error instead of the current warning saying that extraction in a module is "experimental".

The point is to clarify what should and what shouldn't be expected to work, as here are people who are unsure about the status of things unless they take the time to dig into it.

@JasonGross
Copy link
Member

Not that I want to play Cassandra, but asking to be able to extract definitions in the middle of a module does look like giving a stick to get beaten with...

What? Why? Extraction should absolutely work in the middle of a module (though feel free to disallow it in the middle of Module Types and inside functors, i.e., modules with non-zero numbers of parameters). Forbidding extraction inside modules will break my bug minimizer, which relies on modules to replicate the effect of file and directory structure within a single file.

@maximedenes
Copy link
Member

Not that I want to play Cassandra, but asking to be able to extract definitions in the middle of a module does look like giving a stick to get beaten with...

In the context of QuickChick, I can imagine you sometimes want to run a test (which uses extraction) while writing a module.

TBH, I don't see why it would be difficult to support (especially if we forbid it in functors, like @JasonGross says). What risk do you have in mind?

@ppedrot
Copy link
Member

ppedrot commented Sep 9, 2019

My main concern are functors indeed, although you can still get suspicious behaviours when extracting a module that is supposed to be sealed by a signature. What is the expected semantics of this, or more generally extraction inside a module?

@JasonGross
Copy link
Member

when extracting a module that is supposed to be sealed by a signature. What is the expected semantics of this, or more generally extraction inside a module?

The semantics are the same as if you extract something at top-level in the middle of a file (which I model as just a module). I would be fine if extraction errored inside a sealed module, though I think the better thing is to probably just ignore the sealing while inside the module.

@pi8027
Copy link
Contributor

pi8027 commented Sep 10, 2019

I'd like to share the results of my investigation. Monolithic extraction tries to dump all the definitions in the current toplevel here:

| [] when Option.is_empty dir_opt -> [Lib.current_mp (), toplevel_env ()]

In @Lysxia's example, toplevel_env () is the contents of Top but Lib.current_mp () is Top.M (which indeed must be Top), then extract_structure uses that incorrect module name to reconstruct fully qualified names by make_cst:
let rec extract_structure env mp reso ~all = function
| [] -> []
| (l,SFBconst cb) :: struc ->
(try
let sg = Evd.from_env env in
let vl,recd,struc = factor_fix env sg l cb struc in
let vc = Array.map (make_cst reso mp) vl in
let ms = extract_structure env mp reso ~all struc in
let b = Array.exists Visit.needed_cst vc in
if all || b then
let d = extract_fixpoint env sg vc recd in
if (not b) && (logical_decl d) then ms
else begin Visit.add_decl_deps d; (l,SEdecl d) :: ms end
else ms

So the reconstructed name of f is Top.M.f which cannot be recognized as Top.f by Visit.needed_cst. We need to fix this line 59.

@pi8027
Copy link
Contributor

pi8027 commented Sep 10, 2019

This issue may have been introduced in f6d8fc1. We may need to check other uses of Lib.current_mp in extraction...

@pi8027
Copy link
Contributor

pi8027 commented Sep 10, 2019

Since I checked how the extraction bypasses opaqueness of modules, here, I strongly agree with @ppedrot. But leaving extraction in modules as an experimental function and trying to fix it would also be fine.

@pi8027 pi8027 added the kind: anomaly An uncaught exception has been raised. label Sep 27, 2019
@Alizter Alizter added this to Bugs in Extraction May 31, 2022
herbelin added a commit to herbelin/github-coq that referenced this issue Mar 5, 2023
herbelin added a commit to herbelin/github-coq that referenced this issue Mar 5, 2023
@herbelin
Copy link
Member

herbelin commented Mar 5, 2023

Another example of the anomaly, this time with Recursive Extraction:

Require Extraction.
Definition a:=0.
Module M.
Definition b:=a.
Recursive Extraction b.

@pi8027: I worked from your line 59 above and added a kernel function that "flattens" the environment, i.e. that adds the necessary amount of End to close the current stack of open modules, module type and functors. Together with a few other tunings, this seems now to work, even in functors (see PR #17344). Thanks for the analysis of the issue.

herbelin added a commit to herbelin/github-coq that referenced this issue Mar 17, 2023
herbelin added a commit to herbelin/github-coq that referenced this issue Jun 26, 2023
herbelin added a commit to herbelin/github-coq that referenced this issue Jul 2, 2023
coqbot-app bot added a commit that referenced this issue Jul 5, 2023
Reviewed-by: SkySkimmer
Co-authored-by: SkySkimmer <SkySkimmer@users.noreply.github.com>
@coqbot-app coqbot-app bot added this to the 8.18+rc1 milestone Jul 5, 2023
gares pushed a commit to gares/coq that referenced this issue Jul 7, 2023
gares added a commit to gares/coq that referenced this issue Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: anomaly An uncaught exception has been raised. part: extraction The extraction mechanism.
Projects
Archived in project
Extraction
  
Bugs
Development

Successfully merging a pull request may close this issue.

6 participants