Skip to content

Implicit source positions directory locations #2346

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

Conversation

Enoumy
Copy link
Contributor

@Enoumy Enoumy commented Mar 8, 2024

Adds a -directory cli flag to the compiler

We would like for the implicit source position argument to include the "directory" and not just the basename of the file, for example:

(* in app/foo/main.ml*)
let f ~(src_pos : [%src_pos]) () = src_pos

let pos : string = (f ()).pos_fname

In the above example the value of pos would be "main.ml" (just the basename), when we would instead like it to be "app/foo/main.ml" where "app/foo" is the directory relative to the "root" of the build system (e.g. relative to the location of dune-project)

This feature adds a -directory flag the the build system will provide when it calls the compiler. This feature also uses this flag for the implicit source position arguments.

Testing

I have added a test ocaml/testsuite/tests/typing-implicit-source-positions/location_with_directory.ml that tests that the implicit source position argument uses the directory location. Please let me know if this testing is sufficient, if I should add other tests. Thanks!

Enoumy added 11 commits March 7, 2024 12:01
This feature solely fixes merge conflicts that the implicit
source positions project had after not having been rebased since the
summer.

Sadly, I mistakenly committed a new test! I meant to do it on a
different branch, but I accidentally committed some more changes
after that making it trickier to split... Please let me know if I should
only keep this feature do "merge conflict resolution". Thanks!

Testing
-------
- I ran `make test`, and it passes.
- `make install` works, I migrated bonsai to start using this and it works!
- Added test on let operators.
  - Sadly it doesn't quite work, but I think it would be cool if it did
    work as it would allow codebases that use let+ and let* to get
    source code locations! I am unsure about its sound-ness though and
    left CR src_pos'es/question CR's.

Please let me know if there is other additional testing/ci I should perform. Thanks!

I also performed additional cleanup in this feature from self-questions
I had during the merge. Please let me know if there are any changes I
should perform. Thanks!
- CI seems to run `make ci` instead of just `make test`. I've fixed
  more `make ci` changes, although not all, pushing to let ci run
  in case the current "compiler-libs.common" being missing is due
  to a misconfiguration on my environment/if it also fails on the
  github ci being missing is due to a misconfiguration on my
  environment/if it also fails on the github ci.

- Performed a bootstrap

- Fixed chamelon

- Additionally turned my questions on let operator support into CR
  src_pos:
@Enoumy Enoumy requested a review from Ekdohibs as a code owner March 8, 2024 22:01
@Enoumy Enoumy marked this pull request as draft March 8, 2024 22:01
@Enoumy Enoumy changed the title [DRAFT] Implicit source positions directory locations Implicit source positions directory locations Mar 11, 2024
@Enoumy Enoumy marked this pull request as ready for review March 11, 2024 14:19
Copy link
Collaborator

@goldfirere goldfirere left a comment

Choose a reason for hiding this comment

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

Code looks good. Just want an opinion from a few others before merging.

@@ -1630,6 +1638,7 @@ struct
mk_no_alias_deps F._no_alias_deps;
mk_app_funct F._app_funct;
mk_no_app_funct F._no_app_funct;
mk_directory F._directory;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need this in ocamldoc, which is basically deprecated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed!

Copy link
Collaborator

@goldfirere goldfirere left a comment

Choose a reason for hiding this comment

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

Turns out @Enoumy was already on the "asking around" part. Let's merge.

@goldfirere goldfirere merged commit 41edc04 into ocaml-flambda:implicit-source-positions-2 Mar 12, 2024
Enoumy added a commit that referenced this pull request Mar 28, 2024
* Start of implicit-source-positions

Expand [%src_pos] to hard-coded record (#1565)

* Update codeowners

* Revert "Update codeowners"

This reverts commit 313cb2d.

* Start

* backup

* Hacky checkpoint with hard-coded record

* Rename test

* Shadowing works as intended

* Simpler test case

* Fix label layouts

* Change name to lexing_position, fix record layout, add to shadowing test

* Add to test

* Formatting

* Tests, fix predef lexing_position layout, add CR for untypeast shadowing

* Fix test

* Fix untracked file

* Revert "Fix untracked file"

This reverts commit 2a18a6a.

* reformat

* Bootstrap, with predef lexing_position

---------

Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com>

Separate `arg_label`s for Parse and Typed trees (#1589)

* src_pos in fn decls and defns

* Revert is_optional modifications for now

* src_pos shouldn't work in arbitrary places

* Tidy code

* Whitespace

* Add test

* Comment on documentation

* Whitespace

* more whitespace

* Amend test to use Position

* Correctly print [%src_pos] rather than lexing_position and fix corresponding test

* Store hard-coded string in variable

* Separate parsed and typed labels

* Delete fn_decl_and_defn.ml

* Delete invalid_usages.ml

* Minimize diffs in PR

* Minimize diffs in PR

* Update named_function.ml for less diffs

* Clean up

* Put arg_label in Outcome tree

* Revert "Put arg_label in Outcome tree"

This reverts commit cd5ace5.

* is_optional for Parsetree arg_labels

* Restore Asttypes.arg_label, re-export arg_label from Types

* Document distinction between arg_labels

* Fix whitespace

* Add todo comment

Add `arg_label` to `Otyp_arrow` (#1644)

* Put arg_label in Outcome tree

* Add comment for Outcometree.arg_label

`[%src_pos]` in fn decls and defns (#1579)

* src_pos in fn decls and defns

* Revert is_optional modifications for now

* src_pos shouldn't work in arbitrary places

* Tidy code

* Whitespace

* Add test

* Comment on documentation

* Whitespace

* more whitespace

* Amend test to use Position

* Correctly print [%src_pos] rather than lexing_position and fix corresponding test

* Store hard-coded string in variable

* Hacky outcometree printing without creating nontrivial node

* Update documentation for Position labels

* Update tests

* Fix erroneously printing lexing_position for Position arguments

* Clean up code

* Reconstruct constraint for Position arguments in Untypeast

* Update tests

* Move comments around

* Slightly prettify

* Use Location.none in Untypeast reconstructed extensions

* Add Ttyp_src_pos to Typedtree

* Add CR

* Comments

* Clean up code

* Update comment in printtyp.ml

* Add documentation for Ttyp_src_pos, remove extraneous comments

* *synonyms.ml

* src_pos in fn decls and defns

* Revert is_optional modifications for now

* src_pos shouldn't work in arbitrary places

* Tidy code

* Whitespace

* Add test

* Comment on documentation

* Whitespace

* Correctly print [%src_pos] rather than lexing_position and fix corresponding test

* Store hard-coded string in variable

* Hacky outcometree printing without creating nontrivial node

* Update documentation for Position labels

* Update tests

* Fix erroneously printing lexing_position for Position arguments

* Clean up code

* Reconstruct constraint for Position arguments in Untypeast

* Update tests

* Move comments around

* Slightly prettify

* Use Location.none in Untypeast reconstructed extensions

* Add Ttyp_src_pos to Typedtree

* Add CR

* Comments

* Clean up code

* Add documentation for Ttyp_src_pos, remove extraneous comments

* *synonyms.ml

* Merge cleanup

* Add label to Octy_arrow

* Rename function

* transl_label checks for Position

* Pass None to transl_label for classes

* Add comment

* Delete comment

* Consider Position when approximating type

* Add tests for application, recursion

* Error messages mention Position

* Prettify

* Comments

* Rename function

* Add comment

* Remove extraneous calls to label translation

* Test type-based disambiguation

* Add comment for fn app labels

* Add commuting tests

* Remove duplicated src_pos match logic

* Reduce instances of src_pos string

* src_pos in fn decls and defns

* src_pos shouldn't work in arbitrary places

* Whitespace

* Correctly print [%src_pos] rather than lexing_position and fix corresponding test

* Store hard-coded string in variable

* Hacky outcometree printing without creating nontrivial node

* Clean up code

* Slightly prettify

* Add CR

* Clean up code

* Update comment in printtyp.ml

* Merge cleanup

* Add label to Octy_arrow

* Rename function

* transl_label checks for Position

* Pass None to transl_label for classes

* Add comment

* Delete comment

* Consider Position when approximating type

* Add tests for application, recursion

* Error messages mention Position

* Prettify

* Comments

* Rename function

* Add comment

* Remove extraneous calls to label translation

* Test type-based disambiguation

* Add comment for fn app labels

* Add commuting tests

* Remove duplicated src_pos match logic

* Reduce instances of src_pos string

* Make things is_optional again

* Add commuting tests

* Add transl_label_from_pat

* Whitespace

* Parenthesize src_pos in error message

* Fix test

Create `Lexing.position` values when `[%src_pos]` in expressions (#1661)

* Everything

* checkpoint

Construct lambda for src_pos directly

* Revert now unneeded changes to predef

* Clean up comments, whitespace

Implicitly supply source position arguments (#1671)

* Everything

* Apply position arguments when expected type is nothing, refactor creating src_pos exprs

* Add warning instead of modifying existing one

* Fix test

* Move test

* Resolved comments

Clearer classic mode label equivalence checks (#1692)

* Everything

* Apply position arguments when expected type is nothing, refactor creating src_pos exprs

* Add warning instead of modifying existing one

* Fix test

* Move test

* Resolved comments

* Classic mode equivalence check

* Refactor

* Implicit source position merge conflicts (#2275)

* Implicit Source Positions Conflict resolution

This feature solely fixes merge conflicts that the implicit
source positions project had after not having been rebased since the
summer.

Sadly, I mistakenly committed a new test! I meant to do it on a
different branch, but I accidentally committed some more changes
after that making it trickier to split... Please let me know if I should
only keep this feature do "merge conflict resolution". Thanks!

Testing
-------
- I ran `make test`, and it passes.
- `make install` works, I migrated bonsai to start using this and it works!
- Added test on let operators.
  - Sadly it doesn't quite work, but I think it would be cool if it did
    work as it would allow codebases that use let+ and let* to get
    source code locations! I am unsure about its sound-ness though and
    left CR src_pos'es/question CR's.

Please let me know if there is other additional testing/ci I should perform. Thanks!

I also performed additional cleanup in this feature from self-questions
I had during the merge. Please let me know if there are any changes I
should perform. Thanks!

* More cleanup after self-review

* Made CI on Github pass

- CI seems to run `make ci` instead of just `make test`. I've fixed
  more `make ci` changes, although not all, pushing to let ci run
  in case the current "compiler-libs.common" being missing is due
  to a misconfiguration on my environment/if it also fails on the
  github ci being missing is due to a misconfiguration on my
  environment/if it also fails on the github ci.

- Performed a bootstrap

- Fixed chamelon

- Additionally turned my questions on let operator support into CR
  src_pos:

* Removed empty file added accidentally

* Updated let_operator support comment

* Widened question on chamelon compatibility

* Implicit source positions object system support (#2307)

* Implicit Source Positions Conflict resolution

This feature solely fixes merge conflicts that the implicit
source positions project had after not having been rebased since the
summer.

Sadly, I mistakenly committed a new test! I meant to do it on a
different branch, but I accidentally committed some more changes
after that making it trickier to split... Please let me know if I should
only keep this feature do "merge conflict resolution". Thanks!

Testing
-------
- I ran `make test`, and it passes.
- `make install` works, I migrated bonsai to start using this and it works!
- Added test on let operators.
  - Sadly it doesn't quite work, but I think it would be cool if it did
    work as it would allow codebases that use let+ and let* to get
    source code locations! I am unsure about its sound-ness though and
    left CR src_pos'es/question CR's.

Please let me know if there is other additional testing/ci I should perform. Thanks!

I also performed additional cleanup in this feature from self-questions
I had during the merge. Please let me know if there are any changes I
should perform. Thanks!

* Moves changes from original class-type support branch into a rebased branch

* Removed lingering merge conflict markers

* More tests + manually moved a commit from the original class type branch

- For some reason I originally missed a commit that typed the argument
  on classes from the original branch. This feature also grabs the
  tests. Some of my questions revolving `Principal` are no longer
  needed as they seem to have disappeared! I suspect that `make ci`
  now passing in the parent feature is partly/transitively responsible
  somehow for `Principal` now no longer showing up.

* Fixed incorrect merging of invalid_usages.ml

* Removes weird whitespace observed after self-review

* More tests! Found out that application on an inheritance call is unhandled!

* Explicit passing positional argument in a pcl_apply works, erasure still does not.

* Added Pcl_apply support

* More cleanup + removed a question cr from the parent feature.

* More tests. Found another bug! Class type arrows seeem to be left untranslated...

* Fixed type annotation bug + added more tests

* Removed duplicated test

* Added more tests + fixed weird whitespace

* Added question on the two class system environments (val_env vs. met_env)

* Deduplicated more code after self-review

* minor whitespace change

* Removed resolved question CR and addressed new CRs

* Implicit source positions directory locations (#2346)

* Implicit Source Positions Conflict resolution

This feature solely fixes merge conflicts that the implicit
source positions project had after not having been rebased since the
summer.

Sadly, I mistakenly committed a new test! I meant to do it on a
different branch, but I accidentally committed some more changes
after that making it trickier to split... Please let me know if I should
only keep this feature do "merge conflict resolution". Thanks!

Testing
-------
- I ran `make test`, and it passes.
- `make install` works, I migrated bonsai to start using this and it works!
- Added test on let operators.
  - Sadly it doesn't quite work, but I think it would be cool if it did
    work as it would allow codebases that use let+ and let* to get
    source code locations! I am unsure about its sound-ness though and
    left CR src_pos'es/question CR's.

Please let me know if there is other additional testing/ci I should perform. Thanks!

I also performed additional cleanup in this feature from self-questions
I had during the merge. Please let me know if there are any changes I
should perform. Thanks!

* More cleanup after self-review

* Made CI on Github pass

- CI seems to run `make ci` instead of just `make test`. I've fixed
  more `make ci` changes, although not all, pushing to let ci run
  in case the current "compiler-libs.common" being missing is due
  to a misconfiguration on my environment/if it also fails on the
  github ci being missing is due to a misconfiguration on my
  environment/if it also fails on the github ci.

- Performed a bootstrap

- Fixed chamelon

- Additionally turned my questions on let operator support into CR
  src_pos:

* Removed empty file added accidentally

* Updated let_operator support comment

* Widened question on chamelon compatibility

* Moves changes from original class-type support branch into a rebased branch

* Created branch with working changes for directory positions.

* Added a test sanity checking being able to pass in the flag.

* Changed test to properly test that the right basename is supplied

* Removed merge conflict markers upon self review.

* Removed lingering merge artifacts upon self review

* Renamed flag from -dir to -directory

* Fix typo

* Removed directory flag from ocamldoc options

* [Implicit Source Positions] - Better Error Messages (#2364)

* Rename src_pos -> call_pos

Also left some self notes regarding the remaining CR src_pos

* Improved error messages for %call_pos

* Addressed a CR src_pos and removed an already addressed CR src_pos

* Cleanup after self-review

* Updated missing rename after self-review

* Removed addressed c r questions

* Fixed merge conflicts. `make ci` passes locally

* bootstrap

---------

Co-authored-by: jose r <45022810+Enoumy@users.noreply.github.com>
Co-authored-by: enoumy <enoumy@gmail.com>
lukemaurer pushed a commit to lukemaurer/flambda-backend that referenced this pull request Oct 23, 2024
* Start of implicit-source-positions

Expand [%src_pos] to hard-coded record (ocaml-flambda#1565)

* Update codeowners

* Revert "Update codeowners"

This reverts commit 313cb2d.

* Start

* backup

* Hacky checkpoint with hard-coded record

* Rename test

* Shadowing works as intended

* Simpler test case

* Fix label layouts

* Change name to lexing_position, fix record layout, add to shadowing test

* Add to test

* Formatting

* Tests, fix predef lexing_position layout, add CR for untypeast shadowing

* Fix test

* Fix untracked file

* Revert "Fix untracked file"

This reverts commit 2a18a6a.

* reformat

* Bootstrap, with predef lexing_position

---------

Co-authored-by: Richard Eisenberg <reisenberg@janestreet.com>

Separate `arg_label`s for Parse and Typed trees (ocaml-flambda#1589)

* src_pos in fn decls and defns

* Revert is_optional modifications for now

* src_pos shouldn't work in arbitrary places

* Tidy code

* Whitespace

* Add test

* Comment on documentation

* Whitespace

* more whitespace

* Amend test to use Position

* Correctly print [%src_pos] rather than lexing_position and fix corresponding test

* Store hard-coded string in variable

* Separate parsed and typed labels

* Delete fn_decl_and_defn.ml

* Delete invalid_usages.ml

* Minimize diffs in PR

* Minimize diffs in PR

* Update named_function.ml for less diffs

* Clean up

* Put arg_label in Outcome tree

* Revert "Put arg_label in Outcome tree"

This reverts commit cd5ace5.

* is_optional for Parsetree arg_labels

* Restore Asttypes.arg_label, re-export arg_label from Types

* Document distinction between arg_labels

* Fix whitespace

* Add todo comment

Add `arg_label` to `Otyp_arrow` (ocaml-flambda#1644)

* Put arg_label in Outcome tree

* Add comment for Outcometree.arg_label

`[%src_pos]` in fn decls and defns (ocaml-flambda#1579)

* src_pos in fn decls and defns

* Revert is_optional modifications for now

* src_pos shouldn't work in arbitrary places

* Tidy code

* Whitespace

* Add test

* Comment on documentation

* Whitespace

* more whitespace

* Amend test to use Position

* Correctly print [%src_pos] rather than lexing_position and fix corresponding test

* Store hard-coded string in variable

* Hacky outcometree printing without creating nontrivial node

* Update documentation for Position labels

* Update tests

* Fix erroneously printing lexing_position for Position arguments

* Clean up code

* Reconstruct constraint for Position arguments in Untypeast

* Update tests

* Move comments around

* Slightly prettify

* Use Location.none in Untypeast reconstructed extensions

* Add Ttyp_src_pos to Typedtree

* Add CR

* Comments

* Clean up code

* Update comment in printtyp.ml

* Add documentation for Ttyp_src_pos, remove extraneous comments

* *synonyms.ml

* src_pos in fn decls and defns

* Revert is_optional modifications for now

* src_pos shouldn't work in arbitrary places

* Tidy code

* Whitespace

* Add test

* Comment on documentation

* Whitespace

* Correctly print [%src_pos] rather than lexing_position and fix corresponding test

* Store hard-coded string in variable

* Hacky outcometree printing without creating nontrivial node

* Update documentation for Position labels

* Update tests

* Fix erroneously printing lexing_position for Position arguments

* Clean up code

* Reconstruct constraint for Position arguments in Untypeast

* Update tests

* Move comments around

* Slightly prettify

* Use Location.none in Untypeast reconstructed extensions

* Add Ttyp_src_pos to Typedtree

* Add CR

* Comments

* Clean up code

* Add documentation for Ttyp_src_pos, remove extraneous comments

* *synonyms.ml

* Merge cleanup

* Add label to Octy_arrow

* Rename function

* transl_label checks for Position

* Pass None to transl_label for classes

* Add comment

* Delete comment

* Consider Position when approximating type

* Add tests for application, recursion

* Error messages mention Position

* Prettify

* Comments

* Rename function

* Add comment

* Remove extraneous calls to label translation

* Test type-based disambiguation

* Add comment for fn app labels

* Add commuting tests

* Remove duplicated src_pos match logic

* Reduce instances of src_pos string

* src_pos in fn decls and defns

* src_pos shouldn't work in arbitrary places

* Whitespace

* Correctly print [%src_pos] rather than lexing_position and fix corresponding test

* Store hard-coded string in variable

* Hacky outcometree printing without creating nontrivial node

* Clean up code

* Slightly prettify

* Add CR

* Clean up code

* Update comment in printtyp.ml

* Merge cleanup

* Add label to Octy_arrow

* Rename function

* transl_label checks for Position

* Pass None to transl_label for classes

* Add comment

* Delete comment

* Consider Position when approximating type

* Add tests for application, recursion

* Error messages mention Position

* Prettify

* Comments

* Rename function

* Add comment

* Remove extraneous calls to label translation

* Test type-based disambiguation

* Add comment for fn app labels

* Add commuting tests

* Remove duplicated src_pos match logic

* Reduce instances of src_pos string

* Make things is_optional again

* Add commuting tests

* Add transl_label_from_pat

* Whitespace

* Parenthesize src_pos in error message

* Fix test

Create `Lexing.position` values when `[%src_pos]` in expressions (ocaml-flambda#1661)

* Everything

* checkpoint

Construct lambda for src_pos directly

* Revert now unneeded changes to predef

* Clean up comments, whitespace

Implicitly supply source position arguments (ocaml-flambda#1671)

* Everything

* Apply position arguments when expected type is nothing, refactor creating src_pos exprs

* Add warning instead of modifying existing one

* Fix test

* Move test

* Resolved comments

Clearer classic mode label equivalence checks (ocaml-flambda#1692)

* Everything

* Apply position arguments when expected type is nothing, refactor creating src_pos exprs

* Add warning instead of modifying existing one

* Fix test

* Move test

* Resolved comments

* Classic mode equivalence check

* Refactor

* Implicit source position merge conflicts (ocaml-flambda#2275)

* Implicit Source Positions Conflict resolution

This feature solely fixes merge conflicts that the implicit
source positions project had after not having been rebased since the
summer.

Sadly, I mistakenly committed a new test! I meant to do it on a
different branch, but I accidentally committed some more changes
after that making it trickier to split... Please let me know if I should
only keep this feature do "merge conflict resolution". Thanks!

Testing
-------
- I ran `make test`, and it passes.
- `make install` works, I migrated bonsai to start using this and it works!
- Added test on let operators.
  - Sadly it doesn't quite work, but I think it would be cool if it did
    work as it would allow codebases that use let+ and let* to get
    source code locations! I am unsure about its sound-ness though and
    left CR src_pos'es/question CR's.

Please let me know if there is other additional testing/ci I should perform. Thanks!

I also performed additional cleanup in this feature from self-questions
I had during the merge. Please let me know if there are any changes I
should perform. Thanks!

* More cleanup after self-review

* Made CI on Github pass

- CI seems to run `make ci` instead of just `make test`. I've fixed
  more `make ci` changes, although not all, pushing to let ci run
  in case the current "compiler-libs.common" being missing is due
  to a misconfiguration on my environment/if it also fails on the
  github ci being missing is due to a misconfiguration on my
  environment/if it also fails on the github ci.

- Performed a bootstrap

- Fixed chamelon

- Additionally turned my questions on let operator support into CR
  src_pos:

* Removed empty file added accidentally

* Updated let_operator support comment

* Widened question on chamelon compatibility

* Implicit source positions object system support (ocaml-flambda#2307)

* Implicit Source Positions Conflict resolution

This feature solely fixes merge conflicts that the implicit
source positions project had after not having been rebased since the
summer.

Sadly, I mistakenly committed a new test! I meant to do it on a
different branch, but I accidentally committed some more changes
after that making it trickier to split... Please let me know if I should
only keep this feature do "merge conflict resolution". Thanks!

Testing
-------
- I ran `make test`, and it passes.
- `make install` works, I migrated bonsai to start using this and it works!
- Added test on let operators.
  - Sadly it doesn't quite work, but I think it would be cool if it did
    work as it would allow codebases that use let+ and let* to get
    source code locations! I am unsure about its sound-ness though and
    left CR src_pos'es/question CR's.

Please let me know if there is other additional testing/ci I should perform. Thanks!

I also performed additional cleanup in this feature from self-questions
I had during the merge. Please let me know if there are any changes I
should perform. Thanks!

* Moves changes from original class-type support branch into a rebased branch

* Removed lingering merge conflict markers

* More tests + manually moved a commit from the original class type branch

- For some reason I originally missed a commit that typed the argument
  on classes from the original branch. This feature also grabs the
  tests. Some of my questions revolving `Principal` are no longer
  needed as they seem to have disappeared! I suspect that `make ci`
  now passing in the parent feature is partly/transitively responsible
  somehow for `Principal` now no longer showing up.

* Fixed incorrect merging of invalid_usages.ml

* Removes weird whitespace observed after self-review

* More tests! Found out that application on an inheritance call is unhandled!

* Explicit passing positional argument in a pcl_apply works, erasure still does not.

* Added Pcl_apply support

* More cleanup + removed a question cr from the parent feature.

* More tests. Found another bug! Class type arrows seeem to be left untranslated...

* Fixed type annotation bug + added more tests

* Removed duplicated test

* Added more tests + fixed weird whitespace

* Added question on the two class system environments (val_env vs. met_env)

* Deduplicated more code after self-review

* minor whitespace change

* Removed resolved question CR and addressed new CRs

* Implicit source positions directory locations (ocaml-flambda#2346)

* Implicit Source Positions Conflict resolution

This feature solely fixes merge conflicts that the implicit
source positions project had after not having been rebased since the
summer.

Sadly, I mistakenly committed a new test! I meant to do it on a
different branch, but I accidentally committed some more changes
after that making it trickier to split... Please let me know if I should
only keep this feature do "merge conflict resolution". Thanks!

Testing
-------
- I ran `make test`, and it passes.
- `make install` works, I migrated bonsai to start using this and it works!
- Added test on let operators.
  - Sadly it doesn't quite work, but I think it would be cool if it did
    work as it would allow codebases that use let+ and let* to get
    source code locations! I am unsure about its sound-ness though and
    left CR src_pos'es/question CR's.

Please let me know if there is other additional testing/ci I should perform. Thanks!

I also performed additional cleanup in this feature from self-questions
I had during the merge. Please let me know if there are any changes I
should perform. Thanks!

* More cleanup after self-review

* Made CI on Github pass

- CI seems to run `make ci` instead of just `make test`. I've fixed
  more `make ci` changes, although not all, pushing to let ci run
  in case the current "compiler-libs.common" being missing is due
  to a misconfiguration on my environment/if it also fails on the
  github ci being missing is due to a misconfiguration on my
  environment/if it also fails on the github ci.

- Performed a bootstrap

- Fixed chamelon

- Additionally turned my questions on let operator support into CR
  src_pos:

* Removed empty file added accidentally

* Updated let_operator support comment

* Widened question on chamelon compatibility

* Moves changes from original class-type support branch into a rebased branch

* Created branch with working changes for directory positions.

* Added a test sanity checking being able to pass in the flag.

* Changed test to properly test that the right basename is supplied

* Removed merge conflict markers upon self review.

* Removed lingering merge artifacts upon self review

* Renamed flag from -dir to -directory

* Fix typo

* Removed directory flag from ocamldoc options

* [Implicit Source Positions] - Better Error Messages (ocaml-flambda#2364)

* Rename src_pos -> call_pos

Also left some self notes regarding the remaining CR src_pos

* Improved error messages for %call_pos

* Addressed a CR src_pos and removed an already addressed CR src_pos

* Cleanup after self-review

* Updated missing rename after self-review

* Removed addressed c r questions

* Fixed merge conflicts. `make ci` passes locally

* bootstrap

---------

Co-authored-by: jose r <45022810+Enoumy@users.noreply.github.com>
Co-authored-by: enoumy <enoumy@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants