Skip to content

Merge 5.2.0-minus11 #146

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

Merged
liam923 merged 15 commits into from
Jun 18, 2025
Merged

Merge 5.2.0-minus11 #146

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
7590887
Import ocaml sources for ocaml-flambda/flambda-backend@60158e06115
goldfirere Jun 9, 2025
dd2a500
Merge changes
goldfirere Jun 10, 2025
e174f16
Move Unit_info from parsing to typing
goldfirere Jun 10, 2025
019117e
Now merlin needs Numbers (dependency from Jkind)
goldfirere Jun 10, 2025
669c924
Fix easy errors
goldfirere Jun 10, 2025
2e48759
Harder fixes. Please review.
goldfirere Jun 10, 2025
5f4e47b
More edits that require review
goldfirere Jun 10, 2025
b8a8b5b
Fix default pattern recovery
liam923 Jun 11, 2025
028a45b
Bump magic numbers
liam923 Jun 11, 2025
0a61a4c
Format files
liam923 Jun 11, 2025
6341c53
Promote some tests
liam923 Jun 11, 2025
cd4428d
Fix test to use new mode syntax
liam923 Jun 11, 2025
1ccc448
Promote failing parameters test
liam923 Jun 11, 2025
188a9ae
Add new ignored flags
liam923 Jun 11, 2025
4123c5a
Improve construction of unit_info
liam923 Jun 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/analysis/misc_utils.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,5 +63,6 @@ let parse_identifier (config, source) pos =
let is_current_unit comp_unit =
match Env.get_unit_name () with
| Some current_unit ->
String.equal (Compilation_unit.name_as_string current_unit) comp_unit
String.equal (current_unit |> Unit_info.modname |>
Compilation_unit.name_as_string) comp_unit
| None -> false
10 changes: 7 additions & 3 deletions src/kernel/mocaml.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,12 @@ let setup_reader_config config =
let open Mconfig in
let open Clflags in
let ocaml = config.ocaml in
let to_compilation_unit name = Some (Compilation_unit.of_string name) in
Env.set_unit_name (Mconfig.unitname config |> to_compilation_unit);
let to_unit_info name =
Some (name |>
Compilation_unit.of_string |>
Unit_info.make_dummy ~input_name:name)
Copy link
Contributor Author

@liam923 liam923 Jun 11, 2025

Choose a reason for hiding this comment

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

I think this won't break anything, but using make_dummy scares me a bit. It seems like the type of thing that may cause something to break down the line. If we want to call Unit_info.make_with_known_compilation_unit, we can use config.query.filename to fill source_file.

I'm not sure about intf_or_impl - I don't know if Merlin knows that at this point (I'm not sure if Merlin decides this based on solely the filename or whether it's able to parse successfully). And I don't know that the file_prefix argument is for.

Maybe @lukemaurer can chime in on the significance of using make_dummy here, and what the deal is with file_prefix?

Copy link
Contributor

@goldfirere goldfirere Jun 11, 2025

Choose a reason for hiding this comment

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

Yes, I wanted @lukemaurer to review this piece. It was very low confidence.

Copy link
Contributor

@lukemaurer lukemaurer Jun 12, 2025

Choose a reason for hiding this comment

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

Yeah, if there's a real source file then we don't want to be using make_dummy if we can avoid it. make_with_known_compilation_unit makes a lot of sense to me. The intf_or_impl thing is annoying - wish I could remember exactly what it's used for

Copy link
Contributor Author

@liam923 liam923 Jun 13, 2025

Choose a reason for hiding this comment

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

I seem to have lost my permissions to push to this repo. @lukemaurer can you look at the below commit: liam923/merlin@4123c5a

Specifically, I'm not sure what to pass for the file_prefix to Unit_info.make_with_known_compilation_unit.

Copy link
Contributor Author

@liam923 liam923 Jun 16, 2025

Choose a reason for hiding this comment

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

Aha I've gotten my permissions back. Commit 4123c5a is pushed here now.

in
Env.set_unit_name (Mconfig.unitname config |> to_unit_info);
Location.input_name := config.query.filename;
fast := ocaml.unsafe;
classic := ocaml.classic;
Expand All @@ -54,7 +58,7 @@ let setup_reader_config config =

let init_params params =
List.iter params ~f:(fun s ->
Env.register_parameter (s |> Global_module.Name.create_no_args))
Env.register_parameter (s |> Global_module.Parameter_name.of_string))

let setup_typer_config config =
setup_reader_config config;
Expand Down