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.

Sign up for GitHub

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

Jump to bottom

PPX: don't silence warning 20 in fnapply #1166

Merged
merged 4 commits into from
Nov 11, 2024
Merged

PPX: don't silence warning 20 in fnapply #1166

merged 4 commits into from
Nov 11, 2024

Conversation

jchavarri
Copy link
Member

Closes #1165.

This PR should be released together with a change in the documentation of mel.raw over in melange.re to warn users that usage of this extension should always go with explicit type annotations. (I will open an issue in the docs repo when/if this gets merged).

@anmonteiro anmonteiro force-pushed the ppx-dont-silence-warning-20-fnapply branch from 5606733 to 96044c3 Compare November 9, 2024 06:13
@anmonteiro
Copy link
Member

Spent some time this week prototyping a few other solutions to this problem but nothing I tried could be generalized in a nice way :/

so I'm fine with this solution, and I think disabling warning 20 or annotating the %raw expression are fine tradeoffs given the circumstances.

@anmonteiro anmonteiro merged commit 79447a1 into main Nov 11, 2024
5 checks passed
@anmonteiro anmonteiro deleted the ppx-dont-silence-warning-20-fnapply branch November 11, 2024 01:41
@jchavarri jchavarri mentioned this pull request Nov 19, 2024
Open
jchavarri
jchavarri commented Nov 19, 2024
View reviewed changes
Copy link
Member Author

@jchavarri jchavarri left a comment

Choose a reason for hiding this comment

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

Thanks!

Changes.md
@@ -28,6 +28,13 @@ Unreleased
- combined with `[@mel.as ..]` in variant definitions and inline record
payloads, Melange can now express types for discriminated union object
shapes.
- melange-ppx: don't silence warning 20 (`ignored-extra-argument`) for
Copy link
Member Author

Choose a reason for hiding this comment

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

This is breaking change right?

anmonteiro added a commit to anmonteiro/opam-repository that referenced this pull request Feb 10, 2025
@anmonteiro
[new release] melange (5.0.0-53)
377a02f 
CHANGES:

- Make the `unprocessed` alert fatal by default
  ([melange-re/melange#1135](melange-re/melange#1135))
- support `-H` for hidden include dirs in OCaml 5.2
  ([melange-re/melange#1137](melange-re/melange#1137))
- support `[@mel.*]` attributes in uncurried externals
  ([melange-re/melange#1140](melange-re/melange#1140))
- add Worker types to `melange.dom`
  ([melange-re/melange#1147](melange-re/melange#1147))
- Add support for dynamic `import()`
  ([melange-re/melange#1164](melange-re/melange#1164))
- Fix code generation of custom `true` / `false` constructors
  ([melange-re/melange#1175](melange-re/melange#1175))
- Fix code generation of OCaml objects that refers to an init variable in scope
  ([melange-re/melange#1183](melange-re/melange#1183))
- Support `[@mel.as "string"]` in variant definitions
  ([melange-re/melange#884](melange-re/melange#884))
- BREAKING: remove `[@@deriving jsConverter]` for variant definitions
  ([melange-re/melange#884](melange-re/melange#884)). Use `[@mel.as
  "string here"]` instead.
- Support OCaml 5.3 ([melange-re/melange#1168](melange-re/melange#1168))
- Upgrade Stdlib to the OCaml 5.3 Stdlib
  ([melange-re/melange#1182](melange-re/melange#1182))
- Support `[@mel.tag "the_tag"]` in variant definitions
  ([melange-re/melange#1189](melange-re/melange#1189))
  - combined with `[@mel.as ..]` in variant definitions and inline record
    payloads, Melange can now express types for discriminated union object
    shapes.
- melange-ppx: don't silence warning 20 (`ignored-extra-argument`) for
  `%mel.raw` application
  ([melange-re/melange#1166](melange-re/melange#1166)).
  - This change reverts the behavior introduced in
    ([melange-re/melange#915](melange-re/melange#915)).
  - The new recommendation is to annotate `%mel.raw` functions or disable
    warning 20 at the project level.
- ppx,core: propagate internal FFI information via attributes instead of adding
  marshalled data in the native primitive name
  ([melange-re/melange#1222](melange-re/melange#1222))
- melange-ppx: allow `@mel.unwrap` polyvariants not to have a payload
  ([melange-re/melange#1239](melange-re/melange#1239))
- `melange.node`: fix `Buffer.fromString` and add
  `Buffer.fromStringWithEncoding`
  ([melange-re/melange#1246](melange-re/melange#1246))
- `melange.node`: bind to all supported Node.js `Buffer` encodings in
  `Node.Buffer` ([melange-re/melange#1246](melange-re/melange#1246))
- `melange.js`: Add `Js.FormData` with bindings to the
  [FormData](https://developer.mozilla.org/en-US/docs/Web/API/FormData) API
  ([melange-re/melange#1153](melange-re/melange#1153),
  [melange-re/melange#1270](melange-re/melange#1270),
  [melange-re/melange#1281](melange-re/melange#1281)
- `melange.js`: Add `Js.Blob` and `Js.File` with bindings to the
  [Blob](https://developer.mozilla.org/en-US/docs/Web/API/Blob) and
  [File](https://developer.mozilla.org/en-US/docs/Web/API/File) APIs
  ([melange-re/melange#1218](melange-re/melange#1218))
- `melange.js`: add `TypedArray` types at the toplevel in the `Js` module
  ([melange-re/melange#1248](melange-re/melange#1248))
- BREAKING: remove `--mel-g`
  ([melange-re/melange#1234](melange-re/melange#1234))
- runtime(`melange.js`): port `[@mel.send.pipe]` functions to `[@mel.send]`,
  taking advantage of the `@mel.send` + labeled argument improvement (see
  above) ([melange-re/melange#1260](melange-re/melange#1260),
  [melange-re/melange#1264](melange-re/melange#1264),
  [melange-re/melange#1265](melange-re/melange#1265),
  [melange-re/melange#1266](melange-re/melange#1266),
  [melange-re/melange#1280](melange-re/melange#1280),
  [melange-re/melange#1278](melange-re/melange#1278))
- core: fix a crash related to finding constructor names in pattern matching triggered by dune's earlier implementation of `(implicit_transitive_deps false)`
  ([melange-re/melange#1238](melange-re/melange#1238),
  [melange-re/melange#1262](melange-re/melange#1262))
- core: pre-compute the closure param map for functions inlined with
  `--mel-cross-module-opt`
  ([melange-re/melange#1219](melange-re/melange#1219))
- BREAKING: ppx: print the `deprecated` alert for `@@deriving abstract` at the
  declaration site rather than at (all) usages
  ([melange-re/melange#1269](melange-re/melange#1269))
- JS generation: prettify `for` loops
  ([melange-re/melange#1275](melange-re/melange#1275))
- JS generation: improve formatting for `throw` and `return` statements, JS
  objects ([melange-re/melange#1286](melange-re/melange#1286),
  [melange-re/melange#1289](melange-re/melange#1289))
- JS generation: improve formatting for empty return and continue statements
  ([melange-re/melange#1288](melange-re/melange#1288))
- JS generation: remove trailing spaces before commas in `export`
  ([melange-re/melange#1287](melange-re/melange#1287))
- JS generation: remove redundant switch cases branches
  ([melange-re/melange#1295](melange-re/melange#1295))
- JS generation: move space before comma inside `for` definition
  ([melange-re/melange#1296](melange-re/melange#1296))
- JS generation: add space before while loop condition
  ([melange-re/melange#1297](melange-re/melange#1297))
- JS generation: improve indentation of parenthesized blocks
  ([melange-re/melange#1293](melange-re/melange#1293))
- JS generation: add space after constructor comments
  ([melange-re/melange#1294](melange-re/melange#1294))
- JS generation: improve identation of `switch` cases
  ([melange-re/melange#1299](melange-re/melange#1299))
- JS generation: don't generate empty `default:` cases in `switch`
  ([melange-re/melange#1300](melange-re/melange#1300))
- JS generation: emit `module.exports` in CommonJS instead of `exports.x`
  ([melange-re/melange#1314](melange-re/melange#1314))
- JS generation: remove trailing newline after `switch`
  ([melange-re/melange#1313](melange-re/melange#1313))
- ffi: allow annotating `@mel.send` FFI with `@mel.this` to specify which
  parameter should represent the "self" argument.
  ([melange-re/melange#1303](melange-re/melange#1285),
  [melange-re/melange#1310](melange-re/melange#1310))
  - This improvement to the FFI allows expressing more FFI constructs via
    labeled and optionally labeled arguments, e.g. `external foo: value:string
    -> (t [@mel.this]) -> unit = "foo" [@@mel.send]` will now produce
    `t.foo(value)` instead of `value.foo(t)`.
  - It also allows removing usages of `[@mel.send.pipe: t]` in favor of
    `[@mel.send]` with `[@mel.this]`, including when used with `@mel.variadic`.
- ppx: deprecate `[@mel.send.pipe]`
  ([melange-re/melange#1321](melange-re/melange#1321))
- core: fix missed optimization on OCaml versions 5.2 and above, caused by
  [ocaml/ocaml#12236](ocaml/ocaml#12236) generating
  multiple function nodes for `fun a -> fun b -> ...` in the Lambda IR.
anmonteiro added a commit to anmonteiro/opam-repository that referenced this pull request Feb 10, 2025
@anmonteiro
[new release] melange (5.0.0)
6cdb068 
CHANGES:

- Make the `unprocessed` alert fatal by default
  ([melange-re/melange#1135](melange-re/melange#1135))
- support `-H` for hidden include dirs in OCaml 5.2
  ([melange-re/melange#1137](melange-re/melange#1137))
- support `[@mel.*]` attributes in uncurried externals
  ([melange-re/melange#1140](melange-re/melange#1140))
- add Worker types to `melange.dom`
  ([melange-re/melange#1147](melange-re/melange#1147))
- Add support for dynamic `import()`
  ([melange-re/melange#1164](melange-re/melange#1164))
- Fix code generation of custom `true` / `false` constructors
  ([melange-re/melange#1175](melange-re/melange#1175))
- Fix code generation of OCaml objects that refers to an init variable in scope
  ([melange-re/melange#1183](melange-re/melange#1183))
- Support `[@mel.as "string"]` in variant definitions
  ([melange-re/melange#884](melange-re/melange#884))
- BREAKING: remove `[@@deriving jsConverter]` for variant definitions
  ([melange-re/melange#884](melange-re/melange#884)). Use `[@mel.as
  "string here"]` instead.
- Support OCaml 5.3 ([melange-re/melange#1168](melange-re/melange#1168))
- Upgrade Stdlib to the OCaml 5.3 Stdlib
  ([melange-re/melange#1182](melange-re/melange#1182))
- Support `[@mel.tag "the_tag"]` in variant definitions
  ([melange-re/melange#1189](melange-re/melange#1189))
  - combined with `[@mel.as ..]` in variant definitions and inline record
    payloads, Melange can now express types for discriminated union object
    shapes.
- melange-ppx: don't silence warning 20 (`ignored-extra-argument`) for
  `%mel.raw` application
  ([melange-re/melange#1166](melange-re/melange#1166)).
  - This change reverts the behavior introduced in
    ([melange-re/melange#915](melange-re/melange#915)).
  - The new recommendation is to annotate `%mel.raw` functions or disable
    warning 20 at the project level.
- ppx,core: propagate internal FFI information via attributes instead of adding
  marshalled data in the native primitive name
  ([melange-re/melange#1222](melange-re/melange#1222))
- melange-ppx: allow `@mel.unwrap` polyvariants not to have a payload
  ([melange-re/melange#1239](melange-re/melange#1239))
- `melange.node`: fix `Buffer.fromString` and add
  `Buffer.fromStringWithEncoding`
  ([melange-re/melange#1246](melange-re/melange#1246))
- `melange.node`: bind to all supported Node.js `Buffer` encodings in
  `Node.Buffer` ([melange-re/melange#1246](melange-re/melange#1246))
- `melange.js`: Add `Js.FormData` with bindings to the
  [FormData](https://developer.mozilla.org/en-US/docs/Web/API/FormData) API
  ([melange-re/melange#1153](melange-re/melange#1153),
  [melange-re/melange#1270](melange-re/melange#1270),
  [melange-re/melange#1281](melange-re/melange#1281)
- `melange.js`: Add `Js.Blob` and `Js.File` with bindings to the
  [Blob](https://developer.mozilla.org/en-US/docs/Web/API/Blob) and
  [File](https://developer.mozilla.org/en-US/docs/Web/API/File) APIs
  ([melange-re/melange#1218](melange-re/melange#1218))
- `melange.js`: add `TypedArray` types at the toplevel in the `Js` module
  ([melange-re/melange#1248](melange-re/melange#1248))
- BREAKING: remove `--mel-g`
  ([melange-re/melange#1234](melange-re/melange#1234))
- runtime(`melange.js`): port `[@mel.send.pipe]` functions to `[@mel.send]`,
  taking advantage of the `@mel.send` + labeled argument improvement (see
  above) ([melange-re/melange#1260](melange-re/melange#1260),
  [melange-re/melange#1264](melange-re/melange#1264),
  [melange-re/melange#1265](melange-re/melange#1265),
  [melange-re/melange#1266](melange-re/melange#1266),
  [melange-re/melange#1280](melange-re/melange#1280),
  [melange-re/melange#1278](melange-re/melange#1278))
- core: fix a crash related to finding constructor names in pattern matching triggered by dune's earlier implementation of `(implicit_transitive_deps false)`
  ([melange-re/melange#1238](melange-re/melange#1238),
  [melange-re/melange#1262](melange-re/melange#1262))
- core: pre-compute the closure param map for functions inlined with
  `--mel-cross-module-opt`
  ([melange-re/melange#1219](melange-re/melange#1219))
- BREAKING: ppx: print the `deprecated` alert for `@@deriving abstract` at the
  declaration site rather than at (all) usages
  ([melange-re/melange#1269](melange-re/melange#1269))
- JS generation: prettify `for` loops
  ([melange-re/melange#1275](melange-re/melange#1275))
- JS generation: improve formatting for `throw` and `return` statements, JS
  objects ([melange-re/melange#1286](melange-re/melange#1286),
  [melange-re/melange#1289](melange-re/melange#1289))
- JS generation: improve formatting for empty return and continue statements
  ([melange-re/melange#1288](melange-re/melange#1288))
- JS generation: remove trailing spaces before commas in `export`
  ([melange-re/melange#1287](melange-re/melange#1287))
- JS generation: remove redundant switch cases branches
  ([melange-re/melange#1295](melange-re/melange#1295))
- JS generation: move space before comma inside `for` definition
  ([melange-re/melange#1296](melange-re/melange#1296))
- JS generation: add space before while loop condition
  ([melange-re/melange#1297](melange-re/melange#1297))
- JS generation: improve indentation of parenthesized blocks
  ([melange-re/melange#1293](melange-re/melange#1293))
- JS generation: add space after constructor comments
  ([melange-re/melange#1294](melange-re/melange#1294))
- JS generation: improve identation of `switch` cases
  ([melange-re/melange#1299](melange-re/melange#1299))
- JS generation: don't generate empty `default:` cases in `switch`
  ([melange-re/melange#1300](melange-re/melange#1300))
- JS generation: emit `module.exports` in CommonJS instead of `exports.x`
  ([melange-re/melange#1314](melange-re/melange#1314))
- JS generation: remove trailing newline after `switch`
  ([melange-re/melange#1313](melange-re/melange#1313))
- ffi: allow annotating `@mel.send` FFI with `@mel.this` to specify which
  parameter should represent the "self" argument.
  ([melange-re/melange#1303](melange-re/melange#1285),
  [melange-re/melange#1310](melange-re/melange#1310))
  - This improvement to the FFI allows expressing more FFI constructs via
    labeled and optionally labeled arguments, e.g. `external foo: value:string
    -> (t [@mel.this]) -> unit = "foo" [@@mel.send]` will now produce
    `t.foo(value)` instead of `value.foo(t)`.
  - It also allows removing usages of `[@mel.send.pipe: t]` in favor of
    `[@mel.send]` with `[@mel.this]`, including when used with `@mel.variadic`.
- ppx: deprecate `[@mel.send.pipe]`
  ([melange-re/melange#1321](melange-re/melange#1321))
- core: fix missed optimization on OCaml versions 5.2 and above, caused by
  [ocaml/ocaml#12236](ocaml/ocaml#12236) generating
  multiple function nodes for `fun a -> fun b -> ...` in the Lambda IR.
@anmonteiro anmonteiro mentioned this pull request Feb 10, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

PPX: reconsider disabling warning 20 in all functions
2 participants
@jchavarri @anmonteiro