Support for .cppo.ml files

[chrismamo1]

The C Preprocessor for OCaml seems to be gaining traction in quite a few projects (UTop, ppx_deriving, etc.). However, CPPO directives seem to break Merlin. I would like to add a quick fix for this problem, but am unfamiliar with the internals of Merlin. Would any core project maintainers be willing to help me get started on this?

[let-def]

#202 is already about that.
One difficult part was refactoring the frontend to be more modular, which is now done in Merlin_reader.

I see two parts to this work:

• specifying requirements of a generic frontend,
• implementing support for a given preprocessor.

I can look at the first part and help you get started on the second one.

[chrismamo1]

@let-def Sounds great!

I looked through the source and found a file src/kernel/preprocess/lexer_ident.mll which appears as though it would need to be changed to add this sort of functionality. I've made a few of these changes on my own fork but am not sure if this would break existing functionality. I doubt that this would be a breaking change, as I doubt that many people are doing this:

some_object
  #some_method()

[raphael-proust]

@chrismamo1 I can imageine people using this code layout when method chaining. E.g.,

jQuery(".")#find("ul")#find("li")
  #empty()
  #append("something")

Not sure if it exists in practice, but it could.

As long as the cppo parsing can be disabled, I don't think that'd be a problem.

[let-def]

@chrismamo1 this lexer is only used for reconstructing the identifier under the cursor.
The actual lexers for OCaml code are stored in src/ocaml_40x/preprocess/lexer_raw.mll.

But I don't have much time for looking at that right now, sorry.

[maelvls]

@raphael-proust Your example would be working fine with @chrismamo1's change: #empty() would work because

1. Only the words #define, #if... would be accepted by our new lexer rule (not #empty);
2. Method calls imply that there are no spaces between the method name and the left parenthesis. We just have to enforce a space between the directive name and the directive body, and the problem is solved
   • #empty() would be recognized as a method call,
   • #if foo > 2 would be recognized as a cppo directive and thus skipped at lex time.

@chrismamo1 We should also include ( and ) as they are not in identchar nor symbolchar but are necessary in cppo directives.

Note: I cannot find the way to run make preprocess (ocaml 4.04.0), so I cannot test the modified lexer.mll. There is a problem of duplicate rules in parser_raw.mly 😕

[raphael-proust]

I think that, as long as the possible conflicts with objects is documented, it shouldn't be a problem.

Note that the following is valid. It's really not idiomatic though, so I think it should be ok.

# let x = object method define = "define" end ;;
val x : < define : string > = <obj>
# x
  #define
  ;;
- : string = "define"

[maelvls]

Hmm... You are right, I can't avoid grammar clashes in your example! I know so little about the many possibilities of the language - especially object-oriente ones! 😊

As soon as I find out how to work around these duplicate rules in menhir, I'll try to open a PR!

[maelvls]

Does anyone understand the error (running make preprocess in order to re-generate the menhir parser)? The symlinks are configured to ocaml 4.04.0 (but same problem for other versions).

# make preprocess
make -f Makefile.preprocess
menhir --infer --inspection --table --cmly --ocamlc 'ocamlc.opt -I src/ocaml/typer/parsing -I src/ocaml_aux -I src/ocaml/support' src/ocaml/typer/preprocess/parser_raw.mly
File "src/ocaml/typer/preprocess/parser_raw.mly", line 848, characters 0-14:
File "src/ocaml/typer/preprocess/parser_raw.mly", line 2679, characters 0-14:
Error: the nonterminal symbol structure_item is multiply defined.
Only %public symbols can have split definitions.
make[1]: *** [src/ocaml/typer/parser_raw.ml] Error 1
make: *** [preprocess] Error 2

The second declaration of structure_item has the comment (* Lwt *) right above.

I am stuck 😕 Maybe I should use an older version of menhir? (I am using menhir 20170509)

[let-def]

Merlin uses a custom version of Menhir, you can't do the preprocessing without it.
Luckily, latest version of Menhir (the one you have) has the patches merged, but with slightly different semantics and API. We need to port parser to this new version before.

[let-def]

(which I might have the time to do next week)

[let-def]

Done. Current git version should be compatible with the released version of Menhir.

[maelvls]

Thanks, that was fast!! 😀

I still have a slight problem: make preprocess is calling preprocessors/recover/Makefile, in which some targets do not have actual files in preprocessors/recover (fix.ml and gSet.ml). Any idea?
Maybe I don't actually need those to build to get my change working

[let-def]

I forgot to commit those files. That should be fixed now.

[maelvls]

When working on better cppo integration, I have been stuck with merlin not being able to find any .cppo.ml files. With identical files bar.ml and foo.cppo.ml with content

let a () = ()
let () = a ()

and the file commands with the following:

["protocol","version",2]
["checkout","auto","/Users/mvalais/prog/merlin/bug-merlin/bar.ml"]
["tell","start","end","let a () = ()\nlet () = a ()"]
["locate",null,"ml","at",{"line":2,"col":10}]
["checkout","auto","/Users/mvalais/prog/merlin/bug-merlin/foo.cppo.ml"]
["tell","start","end","let a () = ()\nlet () = a ()"]
["locate",null,"ml","at",{"line":2,"col":10}]

and running MERLIN_LOG=/dev/stderr ./ocamlmerlin-server < command

It would give

["return",{"selected":2,"latest":3,"merlin":"The Merlin toolkit version git-864c785c391cee3ad531325357493c2afaf231c1, for Ocaml 4.05.0\n"}]
["return",true]
["return",true]
["return",{"file":"/Users/mvalais/prog/merlin/bug-merlin/bar.ml","pos":{"line":1,"col":0}}]
["return",true]
["return",true]
["return",{"file":"foo.cppo.ml","pos":{"line":1,"col":0}}]

The foo.cppo.ml is not found, as logged in merlin.log:

# 0.05 track_definition - find_source
failed to find "Foo" in source path (fallback = false)
# 0.05 track_definition - find_source
looking for "Foo" in "/Users/mvalais/prog/merlin/bug-merlin"
# 0.05 track_definition - Locate
foo.cppo.ml
# 0.05 frontend - output
[ "return", { "file": "foo.cppo.ml", "pos": { "line": 1, "col": 0 } } ]

After a lot of research, I stubbled upon the PR #474 that adds the SUFFIX thing in .merlin. So the simple trick was to add

SUFFIX .cppo.ml

so that merlin can find the definition of Foo by also including the search of foo.cppo.ml.

Where should I document that?

[let-def]

PR #875 implements a similar feature (OCaml -pp flag).

[XVilka]

Would be nice to have this, since currently merlin-based setup for Vim can't handle files with cppo directives.