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

Context_free: add a special_function' variant #496

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

Octachron
Copy link
Contributor

Parsing Longindent.ts is quite unpractical because Longident.t are used as an IR for different and distinct syntaxic classes. This means that interpreting the string argument of the Context_free.special_function is less trivial than it appears.

To circumvent this issue, this commit takes the view that in complex cases the user themself can still easily specify the identifier that they intended to use.

Thus it sounds sensible to provides a special_function' variant function which takes directly a Longident.t argument for advanced uses.

Copy link
Collaborator

@NathanReb NathanReb left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@NathanReb
Copy link
Collaborator

I missed that the DCO check's unhappy. If you can fix this, I'll merge right after!

@Octachron Octachron force-pushed the stabler_special_function branch from 0477ec1 to 5bff204 Compare May 31, 2024 11:42
@Octachron
Copy link
Contributor Author

The identity confusion and the location of the change entry should be fixed now.

Parsing `Longindent.t` is quite unpractical because Longident.t
are used as an IR for different and distinct syntaxic classes. This
means that interpreting the string argument of the
`Context_free.special_function` is less trivial than it appears.

To circumvent this issue, this commit takes the view that in complex
cases the user themself can still easily specify the identifier that
they intended to use.

Thus it sounds sensible to provides a `special_function'` variant
function which takes directly a `Longident.t` argument for advanced
uses.

Signed-off-by: octachron <[email protected]>
@NathanReb NathanReb force-pushed the stabler_special_function branch from 5bff204 to 0006b5b Compare June 3, 2024 08:35
@NathanReb
Copy link
Collaborator

I just fixed the formatting, will merge when the CI's green.

@NathanReb NathanReb merged commit fd9a7e0 into ocaml-ppx:main Jun 3, 2024
4 of 5 checks passed
NathanReb added a commit to NathanReb/opam-repository that referenced this pull request Jul 22, 2024
- Fix a bug where `Code_path.main_module_name` would not properly remove
  extensions from the filename and therefore return an invalid module name.
  (ocaml-ppx/ppxlib#512, @NathanReb)

- Add `-unused-type-warnings` flag to the driver to allow users to disable
  only the generation of warning 34 silencing structure items when using
  `[@@deriving ...]` on type declarations. (ocaml-ppx/ppxlib#511, @mbarbin, @NathanReb)

- Make `-unused-code-warnings` flag to the driver also controls the generation
  of warning 34 silencing structure items when using `[@@deriving ...]` on type
  declarations. (ocaml-ppx/ppxlib#510, @mbarbin, @NathanReb)

- Driver: Add `-unused-code-warnings=force` command-line flag argument. (ocaml-ppx/ppxlib#490, @mbarbin)

- new functions `Ast_builder.{e,p}list_tail` that take an extra tail
  expression/pattern argument parameter compared to `Ast_builder.{e,p}list`, so
  they can build ASTs like `a :: b :: c` instead of only `[ a; b ]`.
  (ocaml-ppx/ppxlib#498, ocaml-ppx/ppxlib#502, @v-gb, @NathanReb)

- Fix `Longident.parse` so it also handles indexing operators such as
  `.!()`, `.%(;..)<-`, or `Vec.(.%())` (ocaml-ppx/ppxlib#494, @Octachron)

- Add a `special_function'` variant which directly takes a `Longident.t`
  argument to avoid the issue that `Longident.t` cover distinct syntaxic classes
  which cannot be easily parsed by a common parser (ocaml-ppx/ppxlib#496, @Octachron).

- Keep location ranges consistent when migrating `Pexp_function` nodes from 5.2+
  to older versions (ocaml-ppx/ppxlib#504, @jchavarri)

- Fix `-locations-check` behaviour so it is no longer required to pass `-check`
  as well to enable location checks. (ocaml-ppx/ppxlib#506, @NathanReb)

Signed-off-by: Nathan Rebours <[email protected]>
avsm pushed a commit to avsm/opam-repository that referenced this pull request Sep 5, 2024
- Fix a bug where `Code_path.main_module_name` would not properly remove
  extensions from the filename and therefore return an invalid module name.
  (ocaml-ppx/ppxlib#512, @NathanReb)

- Add `-unused-type-warnings` flag to the driver to allow users to disable
  only the generation of warning 34 silencing structure items when using
  `[@@deriving ...]` on type declarations. (ocaml-ppx/ppxlib#511, @mbarbin, @NathanReb)

- Make `-unused-code-warnings` flag to the driver also controls the generation
  of warning 34 silencing structure items when using `[@@deriving ...]` on type
  declarations. (ocaml-ppx/ppxlib#510, @mbarbin, @NathanReb)

- Driver: Add `-unused-code-warnings=force` command-line flag argument. (ocaml-ppx/ppxlib#490, @mbarbin)

- new functions `Ast_builder.{e,p}list_tail` that take an extra tail
  expression/pattern argument parameter compared to `Ast_builder.{e,p}list`, so
  they can build ASTs like `a :: b :: c` instead of only `[ a; b ]`.
  (ocaml-ppx/ppxlib#498, ocaml-ppx/ppxlib#502, @v-gb, @NathanReb)

- Fix `Longident.parse` so it also handles indexing operators such as
  `.!()`, `.%(;..)<-`, or `Vec.(.%())` (ocaml-ppx/ppxlib#494, @Octachron)

- Add a `special_function'` variant which directly takes a `Longident.t`
  argument to avoid the issue that `Longident.t` cover distinct syntaxic classes
  which cannot be easily parsed by a common parser (ocaml-ppx/ppxlib#496, @Octachron).

- Keep location ranges consistent when migrating `Pexp_function` nodes from 5.2+
  to older versions (ocaml-ppx/ppxlib#504, @jchavarri)

- Fix `-locations-check` behaviour so it is no longer required to pass `-check`
  as well to enable location checks. (ocaml-ppx/ppxlib#506, @NathanReb)

Signed-off-by: Nathan Rebours <[email protected]>
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 this pull request may close these issues.

2 participants