Add a `-H` flag, second attempt by ccasin · Pull Request #12246 · ocaml/ocaml

ccasin · 2023-05-15T23:05:36Z

This PR is a reimplementation of #11989, which addresses RFC 31, following a suggestion of @lpw25's to avoid an issue with the previous approach.

It adds a -H flag, which is like -I with two exceptions:

Programs may not directly refer to dependencies added to the search path this way.
-H dependencies go at the very end of the search path.

The previous attempt allowed mixing the -I and -H flags and preserved that order in the search path. This meant, to handle shadowing, some -H modules needed to be added to the typing environment, which had to be updated to track which modules were included in which way. And this potentially creates reproducibility issues along the lines of those fixed in #9991. Leo's suggestion is to instead put the -H modules at the end of the search path, where they can't shadow anything.

I think the result is a win. There is a bit more work in load_path.{ml,mli} to keep track of the -I and -H directories separately, but fewer changes in the type system, and the shadowing problem is avoided.

I've revised and expanded slightly on the testsuite from #11989. I updated ocamldoc and ocamldep to support the new flag, and added it to the manual.

A few things I haven't done:

I haven't updated the cmt format to distinguish the -I and -H directories. It's not clear to me whether this is needed - I took a quick look at merlin, but it doesn't seem to use this part of the cmt.
I haven't updated the toplevel or the debugger to allow for -H. I don't think either is hard to do, if it would be useful.

I have listed @bobot and @gasche as co-authors in the Changes entry as this takes a little code from and is inspired by their attempt, but any mistakes here are mine.

@lpw25 has agreed to review this, though of course feedback from others is welcome. I've broken it into a few commits - the first updates the core system, the second updates the debugger and ocamldoc for compatibility, and the third updates the manual. Edit: but note the changes to the file cache in load_path have been tweaked for efficiency in another commit made after this was posted.

utils/load_path.ml

testsuite/tests/hidden_includes/c1_2.ocamlc.reference

smuenzel · 2023-05-16T02:43:55Z

testsuite/tests/hidden_includes/c3_1.ocamlc.reference

+File "libc/c3.ml", line 1, characters 8-11:
+1 | let x = A.x + 1
+            ^^^
+Error: Unbound module A


Here's a suggestion for an improved error message:

Error: The module A is unbound because it is hidden. (See manual section ....)

Or

Error: The module A cannot be referred to directly, because it is only found in a hidden (-H) include path. (See manual section ...)

We would need to propagate how a particular path was found, but it seems like that should be straightforward.

Changes

ccasin · 2023-05-16T19:34:00Z

Thanks @bobot and @smuenzel!

I've pushed two additional commits. The first slightly reworks the file cache in Load_path and also addresses @smuenzel's suggestion to share some logic in that file. The previous version had a cache of all files and a cache of visible files. This meant visible files were stored in both caches, which was wasteful and probably would be a noticeable performance regression on large builds. The new version has separate visible and hidden caches, and any given file appears only once.

The second commit fixes the typo in the changes file and gives the test reference files more descriptive names.

I'm hesitating to make the suggested change to the error message, because I'm not sure it's actually helpful to users in the typical use case. To say more: The common scenario I'm imagining is that users will write out explicit dependencies for the build system (say in a dune file), and the build system will pass those to the compiler with -I and then compute all the transitive dependencies and pass them with -H. If the program directly refers to something that's only available through -H, the appropriate remedy is for the programmer go to list that module in their dune file. In the current world, they don't ever need to know about the existence of -H - they get the same error as for any module that's not on the load path, and apply the same fix. But with the revised error, they'd have to understand -H and what their build tool is doing about transitive dependencies.

bobot · 2023-05-17T08:45:30Z

It is true that most users will interacts with the -H through a build system that has different concept than -H. However users could be surprised that they can't mention a type that is present in error messages.

-I dir_foo -H dir_bar:

let () = Foo.drink  Foo.banana

  Foo.banana is of type Bar.solid but is expected of type Bar.liquid

In that case one can fix with:

  let () = Foo.drink (Bar.crush Foo.banana)

But get the error Unbound module Bar

Having a different error message would help to directly remember the problem and the fix, moreover the build system can match this error message and add its own hint.

lpw25

A couple of small comments. Looks good to me. Simpler than I was expecting, sorry for not reviewing it sooner.

manual/src/cmds/unified-options.etex

lpw25 · 2023-10-06T17:03:16Z

driver/main_args.ml

+let mk_H f =
+  "-H", Arg.String f,
+  "<dir>  Add <dir> to the list of \"hidden\" include directories\n\
+ \     (Like -I, but the program can not directly reference these dependencies)"


I think is is differently indented by one space compared to the similar entries in this file. Presumably that might make the message indentation off by one too.

The indentation of multiple-line descriptions is a bit inconsistent in this file, but I count six flags where this indentation (5 spaces) is used for subsequent lines: -opaque, -strict-formats, -no-strict-formats, -for-pack, -w, -warn-error. This makes it the most common choice. I checked just now, and it prints like them.

Note: the absence of printing difference comes from the combination of escaped newline and escaped space:

The following two string literals have the same value:

let s1 = "foo\n\ \ bar" let s2 = "foo\n\ \ bar"

ccasin · 2023-10-08T08:07:46Z

I have squashed this PR to one commit, since it no longer had a clean history, and rebased it. There were a few conflicts related to #12389, but they were easily resolved.

I think the only remaining question here is about whether a new error should be added for the special case that the program mentions a module that would be available if they changed a -H flag to a -I flag. I continue to think the current error is best, since most users won't know about -H at all, and the fix is the same as for all other "Unbound module" errors. Regrading @bobot's most recent remark that "users could be surprised that they can't mention a type that is present in error messages" - this is already the case before this PR (see the missing cmi tests).

I think this is fine to merge, once CI passes, if others agree. We can always add a new error later if it turns out this is source of confusion in practice.

gasche

Let me add my voice of support for this PR. It is a notable new feature that I care about (in particular: it is the first significant compiler-level improvement proposed by the Dune people), and the new implementation, following the design recommended by @lpw25, is clearly simpler and better than the one @bobot and myself attempted in #11989. I am in favor of merging.

(I am "approving" to mark support but I only skimmed through the implementation and have not reviewed it in full. I would not consider myself a reviewer, but I understand enough to assess that I am confident in your work and Leo's review.)

Minor points/comments:

I was expecting (secretly hoping?) that there would be a comment somewhere in the compiler codebase that gives a high-level picture of how modules are looked up in the load path / initial environment, to explain the interleaving of -I, Stdlib, -open arguments etc. (I remember asking for a precise description of this when Jérémie Dimino was working on -open.) If this existed, then the present PR should update it. I have been unable to locate it again in the compiler codebase. There is a comment in compmisc.ml that you updated as expected, but there is also a comment at the beginning of load_path.mli that is getting stale (it also lacks a mention of the -cmi-file option I think), could you update it?
In #11989 we wondered about an example where one accesses a "hidden" module through constructor disambiguation, without having to name the hidden module directly.
```
(* A.ml : imported hidden *)
type t = Toto

(* B.mli: imported visible *)
val f : A.t -> unit

(* C.ml: being type-checked *)
let () = B.f Toto
```
I was of the opinion that this example should be rejected, and François that it could be accepted. Can you confirm what the behavior of your implementation is on that example? (My guess would be that it is rejected, but I am not sure.) I am not saying that either behavior is clearly wrong and would be ground to change the code, I think that in this case it is better to "let the code speak" and accept the behavior given by the simplest implementation. But we should be explicit in the discussion about how those edge cases are resolved.
What are the plans for the -cmi-file option, which did not exist when the RFC was originally written? I would expect a -hidden-cmi-file variant. I am not saying that this must be integrated in the present PR, but I would like to see it implemented if there is agreement that it also deserves a hidden variant. What do we think? Are you volunteering to implement it in a followup PR?

gasche · 2023-10-08T09:19:28Z

manual/src/cmds/ocamldoc.etex


+\item["-H" \var{directory}]
+Like "-I", but the "-H" directories are searched last and the program may
+not directly refer to the modules included this way.


The wording here is not great, but nobody cares because these are just ocamldoc options that duplicate compiler options. The current approach (adding meh formulations to a manpage no one reads anymore) is acceptable, but in the medium term it would be nice to get rid of these redundant option descriptions. ( @Octachron may know what would be a good way to do this.)

My general plan for this issue would be to have a unique source of truth for all flags and their documentation and generate all formats and slices from this unique input.

gasche · 2023-10-09T08:49:36Z

A few things I haven't done:

I haven't updated the cmt format to distinguish the -I and -H directories. It's not clear to me whether this is needed - I took a quick look at merlin, but it doesn't seem to use this part of the cmt.

I would be in favor of doing this if possible. It looks like a very simple change, changing cmt_loadpath to take a Load_path.path_info instead of a string list.

By the way, can we bikeshed on path_info a bit? It reads like information on a single path, rather than on all paths. I think that I would prefer if it was named just paths, and returned by get_paths, with a get_visible_paths variant for what is currently get_paths.

I haven't updated the toplevel or the debugger to allow for -H. I don't think either is hard to do, if it would be useful.

I would definitely want to have this in the toplevel. I regularly use the toplevel to test the type-checker, and any mismatch between compilation options and toplevel options is going to block me down the road.

ccasin · 2023-10-09T08:56:19Z

@gasche Thanks for the feedback.

I've adjusted the two comments you mentioned. I did not include a mention of -cmi-file in the Load_path comment, because this option turns out not actually to be implemented in terms of the load path, but rather as special case all the way back in Typemod. For the same reason, it may not be completely straightforward to add a -hidden-cmi-file option, but I'll be happy to investigate this for a future PR.
I tested the module disambiguation example you mentioned. In fact this program is allowed. Leo and I took a look at it just now, and think it would be a bit ugly to change, so we agree with your assessment that we should let the implementation speak for itself, unless it becomes a source of problems.

I will have a go at cmts and the toplevel this morning.

gasche · 2023-10-09T09:00:49Z

Thanks! Maybe it would be useful to have this type-directed-disambiguation corner case in the testsuite.

Octachron · 2023-10-09T11:15:41Z

@gasche The -cmi-file option makes explicit the cmi file associated to a compilation unit being compiled, I don't thunk it makes sense to have a hidden cmi in this context. I believe that you were probably thinking of the -inc option from #9056 which is still in limbo.

ccasin · 2023-10-09T17:08:45Z

I found a few more issues, while working through some of the suggested changes, so I'm pushing 5 more commits - the 4th is significant and needs review in particular. The commits are:

Rename path_info to paths, as suggested. I agree this is a better name.
Update the cmt format for -H. I'm not particularly confident in the change to record_cmt_info in tools/ocamlcmt.ml in this commit. Googling quickly, it wasn't obvious to me whether the annot files created by this function are still in use or a vestige of older versions of ocaml.
Fix tests. It turns out the test wasn't running under make parallel because the test stanza isn't at the top of the file. (Though it would run if you use make one and point at the file - I'm not a sufficient expert on ocamltest to say why the difference). Now at least the test runs consistently.
Fix a rather bad bug in this PR related to the persistent env cache, see below.
Add the suggested test for type-based constructor disambiguation.

The bug fixed in the fourth commit is that, once a -H module had been read in for some legitimate type-checking purpose, it was added to the persistent_structures hashtbl in Persistent_env, and we did not record the fact that it came from a -H directory. So, subsequent uses of this module would work as if it were included with -I. See the test case in this commit for an example that should not typecheck, but does before the commit.

To fix this, the persistent_structures hash table now records whether the cmi was "visible" or "hidden". An alternative would be to never record "hidden" cmis in this structure, but that could result in repetitive and costly loading of cmis. The interface between Load_path and Persistent_env changes a bit to propagate this info. Thankfully, it still stays out of Env.

Finally, with respect to the toplevel: It turns out I was wrong, and -H already works fine with the toplevel. What I did not do is to add a new directive to support interactively adding -H directories. This isn't completely straightforward because such a module should go somewhere in the middle of the Dll search path, and we don't know where, so Dll would need to carry more information or be fully re-initialized each time the directive is executed. I think this is probably not worth doing, but I can if desired.

gasche · 2023-10-09T20:15:56Z

The toplevel behavior you describe is entirely reasonable, and not dealing with extra directives is fine.

@lpw25 would you by chance be available to do another round of review specifically on the new issues that came up and the changes to the implementation?

lpw25

One bug that needs fixing and one opportunity for simplification.

lpw25 · 2023-10-10T13:27:28Z

typing/persistent_env.ml

+  | Found (ps, pm) when allow_hidden || ps.ps_visibility = Load_path.Visible ->
+    (ps, pm)
+  | Found _ -> raise Not_found
  | Missing -> raise Not_found


I think the logic around Missing isn't quite right. If you first try to load a file with allow_hidden is false then you'll set it to Missing in the cache, and that will prevent future lookups even when allow_hidden is true.

For future reference: Leo and I discussed this offline. It should be difficult to get bad behavior out of this, because generally if ~allow_hidden is false we're checking an explicit reference to something in a program, and a failure here will result in a failure of typechecking.

We're not confident that's always the case, though, so I'm pushing a fix which is to only add Missing to the cache if allow_hidden.

An alternative would be to add an argument to Missing to track which kind of lookup has failed. That would have the advantage of avoiding multiple calls to can_load_cmis, but only in the rare case that typechecking succeeds after multiple failing calls to find_pers_struct for the same module with allow_hidden=false. (And the disadvantage of marginal additional complexity and allocation).

The only case that I can think of as potentially problematic would be in Merlin, if they cache failed lookups usefully. (But this assume they watch the filesystem somehow to know when to clean the cache.) Do we know if they do that? Probably not?

(I would be slightly more at ease with caching both with an extra argument to Missing, which we are sure will have no adverse side-effect.)

IIRC they already have to patch this stuff to get their desired behaviour anyway

utils/load_path.ml

gasche · 2023-10-10T13:58:11Z

utils/load_path.mli

+type visibility = Visible | Hidden
+
+val find_normalized_with_visibility : string -> string * visibility
+(** Same as [find_normalized], but also whether the cmi was found in a -I


"but also whether [..]": but also reports/returns/indicates?

gasche · 2023-10-10T19:36:24Z

There is a Windows test failure in the testsuite that I am not sure how to address:

 ... testing 'test.ml' with 1.1.1.1.8.1.1 (check-ocamlc.byte-output) => failed
(compiler output tests/hidden_includes\test\ocamlc.byte\ocamlc.byte.output differs from reference tests\hidden_includes/wrong_include_order.ocamlc.reference: 
@@ -1,3 +1,3 @@
 File "libc/c1.ml", line 1:
-Error: The files "libb/b.cmi" and "liba_alt/a.cmi" make inconsistent assumptions
+Error: The files "libb\b.cmi" and "liba_alt\a.cmi" make inconsistent assumptions
        over interface "A"
)

(several tests fail for the same reason)

I would be tempted to disable those tests under Windows (just add not-windows; at the beginning of the test command, see for inspiration tests/lib-unix/common/fork_cleanup_systhreads.ml). I would like to merge this nice work and I am not too excited about studying how to make the test pass on Windows (using forward slashes instead of backward slashes under Windows more, or post-processing the output, etc.).

ccasin · 2023-10-11T17:25:41Z

@gasche Thanks. I've disabled the relevant tests on windows (I think) and fixed the typo you spotted. I won't have time today to rework Missing (and frankly kind of like the simplicity of the current solution) but I'm happy to do so tomorrow if that is desired.

gasche · 2023-10-11T18:50:33Z

Re. Missing: I have a preference for the cautious route that always caches failures, with a new argument. (I think that this is a more systematic approach for memoization of this function anyway, so the code is simpler / more regular to think about.) On the other hand, I am tired of pulling your leg on this PR. Let me merge know, and you can improve Missing if you feel motivated to do so as a followup PR.

gasche · 2024-02-20T21:16:51Z

It looks this change unwittingly made things harder for ppxlib maintainers, see ocaml-ppx/ppxlib#451 (comment) and then ocaml-ppx/ppxlib#475 . ppxlib is affected by a format change in the ocaml.ppx.context AST payload that comes from 01178bb#diff-e4e6b5f588112250b3281c08994382dce4893de604562364d84737a14d3bec43 . Because this change is only in 5.2 and not an older release, we have an opportunity to consider doing things differently to make ppxlib's life easier.

I know little about this ppx.context things, but here are options that come to mind:

instead of changing the load_path field to be a pair (of string lists) (visible, hidden), we could keep load_path as just a string list (for visible), and add a separate load_path_hidden field
or maybe we could just support a sort of version check on this field, if it is a pair we parse it as (visible, hidden), but if it is just a string list we parse it as visible and set hidden = [].

I suspect that the "support both formats at once" approach may make things more complex, and cause trouble if the ppx contest of a recent release is consumed by something linking against an older release. So from a distance (1) sounds easier, but I know little about these things.

I am also not completely sure of whether this issue is still relevant, or whether the ppxlib people found a change that they like to support arbitrary format differences here.

Octachron · 2024-02-20T21:31:24Z

For full compatibility, it might be better to output load_path as visible @ hidden to keep closer to the previous semantics and add two load_path_visible and load_path_hidden fields.

At the same time, the feedback that I got from @pitag-ha and @NathanReb yesterday was that handling the change in ppxlib worked well enough after adding a compatibility layer.

However, we might want to document an update policy for the ppx context?

gasche · 2024-02-21T14:15:56Z

We discuss this today, there seems to be consensus for @Octachron's proposal (keep the field unchanged, add two new fields that are more precise) if we want to preserve backwards-compatibility. We don't really understand what the ppxlib people prefer at this point (and if it's for good reasons or just to be polite to us), so he will ask again.

NathanReb · 2024-02-21T14:48:27Z

We have just implemented a migration for the new load_path format here that fixes any bug related to this change in ocaml.ppx.context's format.

It's probably worth mentioning that this change only caused trouble in a very specific case, that is: running a ppxlib driver compiled with 5.2 (or higher) on an older marshalled AST.
I implemented this migration because we historically support this use case but it's unclear whether we want to keep doing so. @pitag-ha will confirm but I think this was only relied upon by bucklescript.

In short, there was no strong enough incentive on our side to push for @Octachron's above proposal but a non breaking approach would indeed be our preferred way as we do rely on compiler-libs to read ocaml.ppx.context.

pitag-ha · 2024-02-21T17:17:09Z

Thanks a lot, everyone!

@Octachron and @NathanReb have already explained most of it from the ppxlib perspective. So not much I'll add, just a few confirmations:

For us, it is of course always nicer if everything we use from the compiler - including the PPX context - can be kept stable: The less migrations on our side, the better (less code complexity, less room for bugs or compatibility problems). We haven't hit you up on this, because we're aware that we're using the compiler-libs API with no stability guarantee. It's very nice that you're willing to keep this stable for us! We'll communicate earlier next time if there's a breakage of this kind.

I implemented this migration because we historically support this use case but it's unclear whether we want to keep doing so. @pitag-ha will confirm but I think this was only relied upon by bucklescript.

Yes, it's unclear to us if there're still users with a set-up (i.e. compile the PPX driver with a different compiler than the project) under which this change would break the PPX driver. Everyone who uses dune won't have that problem. However, are there people using a more manual / different build set-up and e.g. a pre-compiled driver with a few standard PPXs?

In short: If you can make the ppx context change non-breaking without much effort (e.g. in the way you've discussed), that's perfect for us. However, if it'd be much effort for you, it's no problem for us if you keep it as is: We can introduce the ppx context migration into ppxlib that @NathanReb has already written. In either case, we know now that you'd help us by keeping things like these stable and, next time, we'll communicate earlier!

ccasin · 2024-02-21T18:29:20Z

I'm sorry that this PR caused issues for ppxlib maintainers! I did update our internally vendored copy of ppxlib to support -H when I rolled it out internally at Jane Street, but I did not know about the backwards compatibility promise. (Sorry also about not more actively participating in the discussion here - I am on vacation this week).

I am only minimally familiar with ppxlib, but my first reaction is that this PR has exposed a pre-existing issue - ppxlib is relying on some compiler-libs functionality not to change, but there is no guarantee of that. My naive instinct is that rather than trying to avoid a breaking change in compiler-libs, we should introduce some explicit versioning into the datastructures ppxlib is reading, so that ppxlib can have a nice migration story that plans for future changes (not that I'm suggesting to start introducing more changes, and in retrospect I clearly should have communicated with ppxlib maintainers about this PR earlier).

Of course, I am not an expert on ppxlib or the versioning policy for compiler-libs, have not taken the time to read the proposed ppxlib patch in detail, and am proposing to create a bunch of work for ppxlib maintainers. So I can easily be convinced this view is incorrect. Either way, I'm happy to help to the extent I can, once I'm back at work next week.

nojb · 2024-08-06T06:29:10Z

Just for info, Dune has added support for this flag: ocaml/dune#10644.

dbuenzli · 2024-08-06T09:51:58Z

Just for info, Dune has added support for this flag: ocaml/dune#10644.

Does it support the exports proposal ?

bobot · 2024-08-06T10:00:27Z

It seems orthogonal. It is the re_export feature of dune, that would be linked to your META exports proposal.

dbuenzli · 2024-08-06T10:08:20Z

Well I think in terms of user experience not features… I think dune could have considered using the name that was proposed there, but oh well…

nojb · 2024-08-06T10:17:50Z

I think dune could have considered using the name that was proposed there, but oh well…

To be clear, the re_export field that @bobot mentions was introduced in Dune in 2019 (see ocaml/dune#2605); much earlier than the Discuss proposal. Its original purpose was to work around some of the issues stemming from the lack of -H in the compiler.

As @bobot mentions, it is likely there is a partial overlap between the semantics of this field and the exports proposal in Discuss, but the overlap may not be total. I will try to read the proposal on Discuss again later and post back here if I manage to understand the situation better.

dbuenzli · 2024-08-06T10:35:40Z

Ok thanks @nojb. Peculiar that no dune dev mentioned the existence of this field in that discussion when we were trying to find a good name.

In any case with the Unicode release around the corner I will soon start shipping META files with this new field populated, so it would perhaps be a good time to agree on the field and the compilation model proposed there.

nojb · 2024-08-16T15:53:36Z

For future reference, this discussion (exports vs (re_export)) continues in ocaml/dune#10830

smuenzel reviewed May 16, 2023

View reviewed changes

utils/load_path.ml Outdated Show resolved Hide resolved

smuenzel reviewed May 16, 2023

View reviewed changes

testsuite/tests/hidden_includes/c1_2.ocamlc.reference Outdated Show resolved Hide resolved

smuenzel reviewed May 16, 2023

View reviewed changes

bobot reviewed May 16, 2023

View reviewed changes

Changes Outdated Show resolved Hide resolved

kit-ty-kate mentioned this pull request Aug 14, 2023

Allow redefinition of polymorphic variants #11483

Open

Octachron assigned gasche Sep 20, 2023

lpw25 approved these changes Oct 6, 2023

View reviewed changes

ccasin force-pushed the dash-h branch from c65aa2e to 7031688 Compare October 8, 2023 07:54

Add -H flag for transitive dependencies

01178bb

ccasin force-pushed the dash-h branch from 7031688 to 01178bb Compare October 8, 2023 08:04

gasche approved these changes Oct 8, 2023

View reviewed changes

Update comments

1291b49

ccasin added 5 commits October 9, 2023 17:51

Rename Load_path.path_info to Load_path.paths

93567a4

update cmt format for -H

45384b3

Fix tests

51ec944

Prevent hidden cmis from incorrectly being used from the penv cache

dc09c0b

Add test for constructor disambiguation

2f72e51

ccasin added 2 commits October 10, 2023 09:10

make depend

8add5d0

Eliminate code duplication

d5ff0a7

lpw25 reviewed Oct 10, 2023

View reviewed changes

Fix additional potential bug in the penv cache

9ce5c9f

gasche reviewed Oct 10, 2023

View reviewed changes

lpw25 approved these changes Oct 10, 2023

View reviewed changes

ccasin added 2 commits October 11, 2023 18:04

Fix comment typo

6143355

Disable tests that print paths on windows

474e96d

ccasin mentioned this pull request Oct 11, 2023

Support platform independent tests with output containing paths #12653

Open

gasche merged commit a912cff into ocaml:trunk Oct 11, 2023

dkalinichenko-js mentioned this pull request Nov 6, 2023

Allow for libraries depending on base to enable dune's (implicit_transitive_deps false) janestreet/base#161

Closed

ccasin mentioned this pull request Nov 8, 2023

Backport of upstream -H implementation oxcaml/oxcaml#1849

Merged

kit-ty-kate mentioned this pull request Nov 30, 2023

Add support for the new -H <dir> argument in OCaml 5.2 ocaml/dune#9333

Closed

hhugo mentioned this pull request Dec 14, 2023

trunk-support supports trunk ocaml-ppx/ppxlib#451

Merged

NathanReb mentioned this pull request Feb 21, 2024

Migrate ocaml ppx context ocaml-ppx/ppxlib#478

Merged

nojb mentioned this pull request May 17, 2024

Add support for -H LexiFi/dune#1

Closed

nojb mentioned this pull request Jun 11, 2024

Add support for -H compiler flag ocaml/dune#10644

Merged

Conversation

ccasin commented May 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ccasin commented May 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bobot commented May 17, 2023

Uh oh!

lpw25 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ccasin Oct 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ccasin commented Oct 8, 2023

Uh oh!

gasche left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gasche commented Oct 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ccasin commented Oct 9, 2023

Uh oh!

gasche commented Oct 9, 2023

Uh oh!

Octachron commented Oct 9, 2023

Uh oh!

ccasin commented Oct 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gasche commented Oct 9, 2023

Uh oh!

lpw25 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gasche commented Oct 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ccasin commented Oct 11, 2023

Uh oh!

gasche commented Oct 11, 2023

Uh oh!

gasche commented Feb 20, 2024

Uh oh!

Octachron commented Feb 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gasche commented Feb 21, 2024

Uh oh!

NathanReb commented Feb 21, 2024

Uh oh!

ccasin commented May 15, 2023 •

edited

Loading

ccasin commented May 16, 2023 •

edited

Loading

ccasin Oct 7, 2023 •

edited

Loading

gasche left a comment •

edited

Loading

gasche commented Oct 9, 2023 •

edited

Loading

ccasin commented Oct 9, 2023 •

edited

Loading

gasche commented Oct 10, 2023 •

edited

Loading

Octachron commented Feb 20, 2024 •

edited

Loading

ccasin commented Feb 21, 2024 •

edited

Loading

bobot commented Aug 6, 2024 •

edited

Loading