Repository navigation
Remove OCaml attribute aliases and the features only OCaml used - #8928
Merged
Merged
Conversation
Why:
The compiler still accepted OCaml spellings of builtin attributes that
nothing in ReScript produces, and some of them only half worked:
`@ocaml.unboxed` changed the type's representation but was ignored by
untagged variants. `@boxed`, `@deprecated_mutable` and
`%extension_constructor` are OCaml features with no use in ReScript.
The `ocaml.*` spellings are now rejected with the bare name to use instead.
`@boxed` and `@deprecated_mutable` are reported as unused attributes, and
`%extension_constructor` as an uninterpreted extension. With it go the
typed tree node `Texp_extension_constructor`, the predefined type
`extension_constructor`, the error "A type cannot be boxed and unboxed at
the same time." and the deprecated-mutation checks. The CMT magic number is
bumped.
Diagnostic added (removed OCaml spelling):
Example:
@ocaml.unboxed
type t = Name(string)
Before (diagnostic): (none, the type is unboxed)
After (diagnostic):
`@ocaml.unboxed` is no longer supported, use `@unboxed` instead.
Diagnostic modified (boxed and unboxed):
Example:
@unboxed @boxed
type t = One(int)
Before (diagnostic):
A type cannot be boxed and unboxed at the same time.
After (diagnostic):
Warning number 101
Unused attribute: @boxed
This attribute has no effect here.
Diagnostic modified (extension constructor):
Example:
exception A
let _ = %extension_constructor(A)
Before (diagnostic): (none, the constructor's slot)
After (diagnostic):
Uninterpreted extension 'extension_constructor'.
Typedtree removed (Texp_extension_constructor):
Before (typed):
expression
Texp_extension_constructor "A"
After (typed): (removed)
Breaking: `@ocaml.deprecated`, `@ocaml.unboxed`, `@ocaml.immediate` and `@ocaml.warnerror` are errors; use `@deprecated`, `@unboxed`, `@immediate` and `@warnerror`. `@ocaml.boxed`, `@ocaml.deprecated_mutable` and `@ocaml.text` are errors; remove them (use a doc comment for `@ocaml.text`). `@boxed` and `@deprecated_mutable` are unused attributes (warning 101, an error by default); remove them. `%extension_constructor` and the type `extension_constructor` are gone.
Review: Why keep `@ocaml.ppwarning`? ppxlib's driver emits `[@@@ocaml.ppwarning]` for lint warnings, so PPXs built with ppxlib still produce it.
Review: Why keep `@immediate`, `@warnerror` and `@ppwarning`? `@immediate` on an abstract type makes comparisons on it compile to `===`/`<`; `@warnerror` turns warnings into errors in a scope; `@ppwarning` is how PPXs report warnings.
Review: Does removing a typed tree constructor break reading older CMTs? The CMT magic is bumped; older CMTs were already unreadable since `Texp_dict` was inserted mid-variant.
Area: compiler/ml, compiler/frontend, analysis
Output: unchanged for programs that use none of the removed spellings
Tests: super_errors fixtures for the removed spellings, unused `@boxed`/`@deprecated_mutable` and `%extension_constructor`; three obsolete fixtures removed; `@ocaml.unboxed` in the reanalyze and gentype tests switched to `@unboxed`
PPX: v0 wire unchanged; `ocaml.error`, `ocaml.warning`, `ocaml.ppwarning` and `ocaml.explicit_arity` still accepted
Editor: hover no longer reads `@ocaml.text` as documentation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo
Signed-off-by: Christoph Knittel <ck@cca.io>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #8928 +/- ##
==========================================
+ Coverage 81.03% 81.11% +0.08%
==========================================
Files 455 455
Lines 63640 63672 +32
==========================================
+ Hits 51573 51650 +77
+ Misses 12067 12022 -45
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8928 |
Why:
`ocaml.warning`, `ocaml.ppwarning` and `ocaml.error` were still read
everywhere because PPXs built on OCaml tooling emit them (ppxlib reports
errors as `[%ocaml.error]` and lint warnings as `[@@@ocaml.ppwarning]`).
Renaming them once, where PPX output enters the compiler, lets every
other reader use the bare name.
The PPX bridge (`Ast_mapper_from0`) renames the three names in its default
attribute and extension mappers, so they are renamed at any depth,
payloads included. `Ast_mapper.extension_of_error` emits `error`, and the
internal `ocaml.warn_on_literal_pattern` that `Predef` puts on `Failure`
and `Invalid_argument` is now `warn_on_literal_pattern`. Written by hand in
source, the prefixed spellings are rejected like the other removed
aliases; the check now also sees attributes on constants.
AST modified (PPX output):
Example:
%%ocaml.error("ppx error")
let x = @ocaml.warning("-3") 1
Before (parse):
structure_item
Pstr_extension "ocaml.error"
attribute "ocaml.warning"
After (parse):
structure_item
Pstr_extension "error"
attribute "warning"
Diagnostic added (written by hand):
Example:
@@ocaml.warning("-32")
Before (diagnostic): (none, warning 32 disabled)
After (diagnostic):
`@ocaml.warning` is no longer supported, use `@warning` instead.
Breaking: `@ocaml.warning`, `@ocaml.ppwarning` and `%ocaml.error` in source are errors; use `@warning`, `@ppwarning` and `%error`. A hand-written `@ocaml.warn_on_literal_pattern` no longer has an effect. PPX output that uses the prefixed names keeps working.
Review: Can a PPX's `ocaml.warning` still reach the error check? Only if the bridge skipped an attribute position; the ounit test covers structure and signature items, type declarations, expressions and attribute payloads, and the ast-mapping fixture covers the parser round trip.
Review: Does warning 52 still fire on `Failure("boom")`? Yes, a new super_errors fixture covers it.
Area: compiler/ml, compiler/frontend, analysis
Output: unchanged
Tests: ounit tests for the bridge renaming and `extension_of_error`; ast-mapping fixture OcamlPrefixedBuiltins; super_errors fixtures for hand-written `@ocaml.warning`, `@ocaml.ppwarning`, `%ocaml.error` and warning 52 on `Failure`; the reanalyze test's `@@ocaml.warning` switched to `@@warning`
PPX: v0 wire unchanged; `ocaml.warning`, `ocaml.ppwarning` and `ocaml.error` from a PPX are renamed to the bare names; `ocaml.explicit_arity` still read by the bridge
Editor: reanalyze reads only `@warning`
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo
Signed-off-by: Christoph Knittel <ck@cca.io>
Why:
Reporting them as unused attributes (warning 101, an error by default)
broke builds over attributes that never changed anything in ReScript.
Diagnostic removed (unused boxed):
Example:
@boxed
type t = One(int)
Before (diagnostic):
Warning number 101
Unused attribute: @boxed
This attribute has no effect here.
After (diagnostic): (none, the attribute is ignored)
Breaking: `@boxed` and `@deprecated_mutable` are no longer interpreted and are silently ignored; remove them.
Area: compiler/frontend
Output: unchanged
Tests: the unused_boxed_deprecated_mutable super_errors fixture is removed
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo
Signed-off-by: Christoph Knittel <ck@cca.io>
Member
Author
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: faaa6bdef0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…CMI magic
Why:
With a `-ppx`, the source tree went through the PPX bridge, which
renames `ocaml.warning`, `ocaml.ppwarning` and `ocaml.error` to the bare
names, before the removed-spelling check ran, so hand-written
`@@ocaml.warning("-32")` was silently accepted. Removing the predefined
type `extension_constructor` shifted the stamps of later predefined
types, but `.cmi` files from before the change were still accepted.
`Cmd_ppx_apply` now runs the removed-spelling check from `Bs_ast_invariant`
on the source before handing it to external PPXs; the check after the PPXs
is unchanged. The CMI magic number is bumped to `Caml1999I039`.
Diagnostic added (with an external PPX):
Example:
@@ocaml.warning("-32")
let value = 1
Before (diagnostic): (none, accepted as `@@warning("-32")`)
After (diagnostic):
`@ocaml.warning` is no longer supported, use `@warning` instead.
Breaking: `.cmi` files built by an earlier compiler are rejected; rebuild them.
Area: compiler/core, compiler/frontend, compiler/ext
Output: unchanged
Tests: ounit test ppx_removed_ocaml_spelling compiles `@@ocaml.warning`, `@ocaml.ppwarning` in an interface and `%ocaml.error` through an identity PPX and expects the errors, and `@@warning` to compile
PPX: v0 wire unchanged; source is checked before it is sent to a PPX, and PPX output is still renamed
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo
Signed-off-by: Christoph Knittel <ck@cca.io>
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0055c90e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The removed `ocaml.*` attributes share one table lookup, which the `@ocaml.unboxed` fixture already covers. Warning 52 on predefined exceptions is covered by `warning_52_fragile_literal_pattern`, and the bridge renaming by the ounit test. Area: tests Output: unchanged Tests: removed super_errors fixtures removed_ocaml_deprecated, removed_ocaml_boxed, removed_ocaml_warning, removed_ocaml_ppwarning and warning_52_failure_literal_pattern, and the ast-mapping fixture OcamlPrefixedBuiltins, each with its expected file Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel <ck@cca.io>
cknitt
marked this pull request as ready for review
October 20, 2026 07:54
cristianoc
approved these changes
Oct 20, 2026
cristianoc
left a comment
cristianoc
left a comment
Collaborator
There was a problem hiding this comment.
Amazing that these are still around after all the rounds of cleanup.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
v13 cleanup: the compiler reads builtin attributes only by their bare names. Hand-written OCaml spellings (
@ocaml.deprecated,@ocaml.unboxed,@ocaml.immediate,@ocaml.warnerror,@ocaml.warning,@ocaml.ppwarning,@ocaml.boxed,@ocaml.deprecated_mutable,@ocaml.text,%ocaml.error) fail with an error naming the bare attribute.PPX output keeps working: the v0 bridge renames
ocaml.warning,ocaml.ppwarningandocaml.error(what ppxlib emits) to the bare names, andAst_mapper.extension_of_erroremitserror. The internalocaml.warn_on_literal_patternis nowwarn_on_literal_pattern.The OCaml-only
@boxedand@deprecated_mutableare no longer interpreted (ignored like any unknown attribute), and%extension_constructoris removed withTexp_extension_constructor, the predefined typeextension_constructorand their errors. The CMT magic is bumped.@immediate,@warnerrorand@ppwarningstay.New tests: ounit tests for the bridge renaming and for rejecting hand-written spellings before an external PPX, and super_errors fixtures for
@ocaml.unboxed,%ocaml.errorand%extension_constructor.🤖 Generated with Claude Code
https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo