Compile flags

[lubegasimon]

addressing #766 .

[shonfeder]

Looking good!

I think som irrelevant commits may have slipped in here, and we need to work out the tests, but it looks like we're parsing the field as desired!

[shonfeder]

I don't think this should be part of the PR.

[lubegasimon]

true, it should not be part of it @shonfeder, am not so certain why such git history reflects in here, I have done git reset <commitID>, but all in vain.

[shonfeder]

I'd either rebase just the relevant changes onto master or create a fresh branch off of master and just cherry pick in the relevant commits from here.

[shonfeder]

I'm not %100 sure what the best way to test this will be, but I think something like the following might work:

• Create a new test file under the inline_tests directory called something like /compiler-flags.t/run.t
• Set up a backend runner there that just works to detect whether certain conspicuous compiler flags have been set
• Set up one or two example targets that build the tests using different compiler flags

Do you have any thoughts for a more elegant test set up @jeremiedimino?

[ghost]

That seems about right. You could also pass a flag that you are sure the compiler doesn't accept and observe the compilation failure.

BTW, the above test is using the OCAMLPATH trick to test that inline test backends work even when installed on the system. But since we already tested that, we don't need to use this method again and we can instead use a locally defined backend. So the shell command could just be: dune runtest dune-file-compile-flags.

[ghost]

Looks like this is the problem. lib.buildable.flags corresponds to the flags attached to the library stanza. These are used to compiler the various modules of the library. Here, we want to use the flags specifically given by the user to compiler the test runner, which should be in info.compile_flags.

[lubegasimon]

which means compile_flags is now of type Ocaml_flags.Spec.t and following this path, I again hit

+  File "dune", line 11, characters 5-18:
+  11 |     (compile_flags -flag-that-is-not-accepted-by-ocaml)))
+            ^^^^^^^^^^^^^
+  Error: Unknown field compile_flags

and you addressed that issue here

[ghost]

Indeed, is it clear how to make progress?

The idea is that when you write let+ <var> = flag "<name>" decoder or and+ <var> = flag "<name>" decoder, you are telling how to parse a field named <name>. decoder explains how to parse the argument of this field. Ocaml_flags.Spec.decode explains how to parse a set of three field and instead we want to parse a single compile_flags field of type Ordered_set_lang.Unexpanded.t

[lubegasimon]

Indeed, is it clear how to make progress?

now it seems clear, thanks.

[lubegasimon]

@jeremiedimino, am afraid, having compile_flags field to be of type Ordered_set_lang.Unexpanded.t will require us to pass lib.buildable.flags as a third arg to Super_context.ocaml_flag ( which expects the arg to be of type Ocaml_flags.Spec.t ), yet we intend to use info.compile_flags as you suggested above. This will call for changes in the type of Super_context.ocaml_flag which will subsequently result in type changes in many files which may cause breaking changes, am afraid.

[ghost]

If you look at Ocaml_flags.Spect.t, it is a set of three things:

• a list of flags to pass to ocamlc
• a list of flags to pass to ocamlopt
• a common list of flags to pass to both ocamlc and ocamlopt

These three fields have the type Ordered_set_lang.Unexpanded.t. Since we are only adding one field compile_flags, it should correspond to a common list of flags to pass to both ocamlc and ocamlopt. So what we need is a way to create an Ocaml_flags.Spec.t value from a list of common flags. You can add a new function in Ocaml_flags.Spec for that.

[lubegasimon]

closed in favour of #3747