Skip to content

Process attributes after Qed/Defined#22118

Open
gmalecha-at-skylabs wants to merge 2 commits into
rocq-prover:masterfrom
SkyLabsAI:gmalecha/attributes-on-hooks
Open

Process attributes after Qed/Defined#22118
gmalecha-at-skylabs wants to merge 2 commits into
rocq-prover:masterfrom
SkyLabsAI:gmalecha/attributes-on-hooks

Conversation

@gmalecha-at-skylabs

@gmalecha-at-skylabs gmalecha-at-skylabs commented Jun 10, 2026

Copy link
Copy Markdown

This PR extends the handling of attributes so that handlers are executed when the term is completed, e.g. at the Qed or Defined.

It uses this feature to implement a plugin that provides an attribute #[hint(db=...,cost=...,visibility=...)]. This should be either incorporated into core Rocq or introduced as a separate plugin.

For now, I'm just looking for feedback, the commits should be independently reviewable. The last commit contains the hint attribute implementation and could probably use more testing.

  • Added / updated test-suite.
  • Added changelog.
  • Opened overlay pull requests.

@coqbot-app coqbot-app Bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Jun 10, 2026
Comment thread vernac/comFixpoint.ml Outdated
@gmalecha-at-skylabs gmalecha-at-skylabs marked this pull request as ready for review June 12, 2026 15:30
@gmalecha-at-skylabs gmalecha-at-skylabs requested a review from a team as a code owner June 12, 2026 15:30
@gmalecha-at-skylabs

Copy link
Copy Markdown
Author

@SkySkimmer What should the next steps here be?

  1. I think it would be really useful to get the core part of the change in so that attributes on Lemma and whatnot work well. The only concern is that you could run into an error on Qed after you do a big proof.
  2. What do you think about the code that actually implements the attribute? Should it be placed in a plugin that is distributed separately?

I'm happy to put more cycles into the development.

@SkySkimmer

Copy link
Copy Markdown
Contributor

I think it would be really useful to get the core part of the change in so that attributes on Lemma and whatnot work well. The only concern is that you could run into an error on Qed after you do a big proof.

That seems fine, there doesn't seem to be a way to get the error earlier anyway.

What do you think about the code that actually implements the attribute? Should it be placed in a plugin that is distributed separately?

I don't see the point of having it be a plugin, it seems like it should be in the statically linked part. But do it as a separate PR since it has much more bikeshed potential.

@gmalecha-at-skylabs gmalecha-at-skylabs force-pushed the gmalecha/attributes-on-hooks branch from 110bc01 to 3034b35 Compare June 15, 2026 16:19
@gmalecha-at-skylabs

Copy link
Copy Markdown
Author

I have removed the plugin code from the PR.

@SkySkimmer SkySkimmer self-assigned this Jun 15, 2026
@SkySkimmer

Copy link
Copy Markdown
Contributor

@coqbot run full ci

@coqbot-app coqbot-app Bot removed the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Jun 15, 2026

(* Programmable-attribute hooks must also fire when a Lemma/Theorem is
completed (at Qed/Defined), not only for Definition. *)
#[print]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I feel like this doesn't test much since the test script doesn't check the output

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'm used to dune output/expect tests, but I think those don't work here. I've added the tests though.

Comment thread test-suite/misc/attributes/theories/attr.v
Comment thread vernac/declare.mli
val make_g : (S.t -> 'a -> 'a) -> 'a g
val make : (S.t -> unit) -> t
val call : ?hook:t -> S.t -> unit
val seq : t -> t -> t

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

how about adding a list version and using that in vernacentries?
I believe it would also mean call can be unexported

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I've added seqs, but call was pre-existing (probably unused though).

@SkySkimmer SkySkimmer added kind: enhancement Enhancement to an existing user-facing feature, tactic, etc. needs: changelog entry This should be documented in doc/changelog. and removed needs: changelog entry This should be documented in doc/changelog. labels Jun 15, 2026
@gmalecha-at-skylabs gmalecha-at-skylabs force-pushed the gmalecha/attributes-on-hooks branch from 3034b35 to 154d476 Compare June 16, 2026 16:58
@coqbot-app coqbot-app Bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Jun 16, 2026
@gmalecha-at-skylabs gmalecha-at-skylabs force-pushed the gmalecha/attributes-on-hooks branch from 154d476 to 40e8800 Compare June 17, 2026 02:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: enhancement Enhancement to an existing user-facing feature, tactic, etc. needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants