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

Please do not depend on autogenerated names of destruct that are sensitive to the file in which records are defined #130

Closed
JasonGross opened this issue Oct 26, 2021 · 2 comments · Fixed by #131

Comments

@JasonGross
Copy link
Contributor

My bug minimizer cannot fully minimize the example at coq/coq#14798 (comment) because

destruct net.
concludes.
destruct nwState. auto.
relies on the fact that the record Verdi.Net.network is defined in some file other than GhostSimulations.v (in this case, in the file Net.v). This is an instance of coq/coq#3523.

Suggested fixes:

-  destruct net.
+  destruct net as [? nwState].
   concludes.
   destruct nwState.

or

   destruct net.
   concludes.
-  destruct nwState.
+  let nwState := match goal with H : data |- _ => H end in destruct nwState.
@palmskog
Copy link
Collaborator

@JasonGross I can apply one of the fixes, but which one do you recommend? (A PR is also welcome.)

@JasonGross
Copy link
Contributor Author

I'd suggest whichever one is more robust to future changes: the former is more robust to the changes in the specific types of the fields and the presence of new context variables, while the latter is more robust to changes in field ordering. Which of these seems more important to be robust to? (Said another way, which better captures the essence of the proof: "destruct the second field of net a second time" or "destruct the first thing in the context of type data"?)
(If they seem equally relevant, I'd lean towards the first one, since it's fewer characters and probably easier for humans to read.)

JasonGross added a commit to JasonGross/verdi that referenced this issue Oct 26, 2021
Fixes uwplse#130 ; see uwplse#130 for more details

Ideally we'd have a way to find and fix all instances like this at once.
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 a pull request may close this issue.

2 participants