Skip to content

Make Nativecode.string_of_kn really injective for good. - #22399

Open
ppedrot wants to merge 1 commit into
rocq-prover:masterfrom
ppedrot:native-compile-fix-name-injectivity
Open

Make Nativecode.string_of_kn really injective for good.#22399
ppedrot wants to merge 1 commit into
rocq-prover:masterfrom
ppedrot:native-compile-fix-name-injectivity

Conversation

@ppedrot

@ppedrot ppedrot commented Aug 24, 2026

Copy link
Copy Markdown
Member

This is critical for soundness of native compilation, as otherwise a definition may overwrite another one. This was not the first time such a problem arised, see 244d7a9.

Fixes #22364: string_of_kn is non-injective.

@ppedrot ppedrot added this to the 9.2.1 milestone Aug 24, 2026
@ppedrot
ppedrot requested review from a team as code owners August 24, 2026 14:09
@ppedrot ppedrot added kind: fix This fixes a bug or incorrect documentation. kind: inconsistency Proof of False accepted by the kernel and/or checker. request: full CI Use this label when you want your next push to trigger a full CI. labels Aug 24, 2026
@coqbot-app coqbot-app Bot removed the request: full CI Use this label when you want your next push to trigger a full CI. label Aug 24, 2026
@SkySkimmer SkySkimmer added the needs: test-suite update Test case should be added to / updated in the test-suite. label Aug 24, 2026
@SkySkimmer

Copy link
Copy Markdown
Contributor

Is this actually injective? seems like both foo_.bar and foo._bar produce foo___bar no?

@JasonGross

Copy link
Copy Markdown
Member

What about mapping _ to _u and . to _d

@ppedrot

ppedrot commented Aug 24, 2026

Copy link
Copy Markdown
Member Author

Is this actually injective?

You're right. I think that for this kind of property I can only trust formal proofs...

@ppedrot

ppedrot commented Aug 24, 2026

Copy link
Copy Markdown
Member Author

Note that there is also a potential issue with the encoding of bound module paths, we prefix them with X and just take the underlying name, this doesn't look very injective to me, but maybe it's not exploitable.

@JasonGross

Copy link
Copy Markdown
Member

Note that there is also a potential issue with the encoding of bound module paths, we prefix them with X and just take the underlying name, this doesn't look very injective to me, but maybe it's not exploitable.

If we follow my suggestion of having all escapes be _ followed by something that is not _, we can prefix them with _X

@ppedrot

ppedrot commented Aug 24, 2026

Copy link
Copy Markdown
Member Author

I'll do something along those lines, it's indeed safer.

@github-actions github-actions Bot added needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. labels Aug 26, 2026
This is critical for soundness of native compilation, as otherwise a
definition may overwrite another one. This was not the first time such
a problem arised, see 244d7a9.

We go for a belt-and-braces approach and try to make all subcomponents
injective, including the modpath printer.

Fixes rocq-prover#22364: string_of_kn is non-injective.
@ppedrot ppedrot added the request: full CI Use this label when you want your next push to trigger a full CI. label Aug 26, 2026
@ppedrot
ppedrot force-pushed the native-compile-fix-name-injectivity branch from 755b784 to b2e9d86 Compare August 26, 2026 14:25
@coqbot-app coqbot-app Bot removed request: full CI Use this label when you want your next push to trigger a full CI. needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. labels Aug 26, 2026
@ppedrot ppedrot removed the needs: test-suite update Test case should be added to / updated in the test-suite. label Aug 26, 2026
@ppedrot

ppedrot commented Aug 26, 2026

Copy link
Copy Markdown
Member Author

This should be ok now. (There is still something potentially fishy with the bound module names unique identifiers, but it's not clear to me how we can be in a situation where both are accessible to the native compiler. Any idea to break that?)

@jasonhokinlove-design

This comment was marked as low quality.

@JasonGross

Copy link
Copy Markdown
Member

how we can be in a situation where both are accessible to the native compiler. Any idea to break that?

Is it not possible to trigger native compute inside a functor?

@ppedrot

ppedrot commented Aug 26, 2026

Copy link
Copy Markdown
Member Author

AFAIU the only way to have two reachable bound module names with different ids is via module substitution, and at this point we don't care anymore about toplevel compilation of constants in native. (Rocq prevents you to syntactically declare twice the same name for module parameters.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: fix This fixes a bug or incorrect documentation. kind: inconsistency Proof of False accepted by the kernel and/or checker.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

string_of_kn is non-injective over module paths (A.B.xA_B.x) → kernel-accepted, axiom-free proof of False in the native_compute lane

4 participants