Skip to content
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

ocamldep should error out if a rewriter returns an error #6795

Closed
vicuna opened this issue Feb 27, 2015 · 7 comments
Closed

ocamldep should error out if a rewriter returns an error #6795

vicuna opened this issue Feb 27, 2015 · 7 comments
Labels
Milestone

Comments

@vicuna
Copy link

vicuna commented Feb 27, 2015

Original bug ID: 6795
Reporter: @whitequark
Assigned to: @diml
Status: closed (set by @xavierleroy on 2017-02-16T14:18:08Z)
Resolution: fixed
Priority: normal
Severity: minor
Target version: 4.03.0+dev / +beta1
Fixed in version: 4.03.0+dev / +beta1
Category: ~DO NOT USE (was: OCaml general)
Has duplicate: #6996
Monitored by: @gasche @hcarty

Bug description

Currently, if a rewriter returns an error, ocamldep blindly processes the [%%ocaml.error] node instead, and returns no dependency information at all, which causes the rest of compilation to fail in unexpected and confusing ways (no module found) if it does in fact depend on dependencies being present.

This happens with my package ppx_import.

@vicuna
Copy link
Author

vicuna commented Feb 27, 2015

Comment author: @gasche

I'm not convinced that being stricter at what we accept won't break existing tooling in subtle way. The safest choice is probably to do exactly as for camlp4 here: what happens if ocamldep is called on a file whose preprocessor rejects?

@vicuna
Copy link
Author

vicuna commented Feb 27, 2015

Comment author: @whitequark

No, that wouldn't work. camlp4 reports the error itself and exits; ppx has to always return a valid AST and rely on the compiler reporting the error.

@vicuna
Copy link
Author

vicuna commented Feb 27, 2015

Comment author: @gasche

I agree on principle on the idea of adding code to ocamldep to detect the specific error-return convention of -ppx. It's just that I don't know how robust or how fragile ocamldep is when dealing with camlp4 preprocessing errors, and I suggest to make it exactly as robust (and fragile) with ppx errors (recognized as such).

@vicuna
Copy link
Author

vicuna commented Feb 27, 2015

Comment author: @whitequark

I believe adding such error handling code to the -ppx branch would do what you want.

@vicuna
Copy link
Author

vicuna commented Dec 17, 2015

Comment author: @diml

When camlp4 fails, ocamldep fails as well but still prints deps:

cat test.ml

let a = Foo.a
let x = -

cat foo.ml

let a = 42

ocamldep -pp camlp4o test.ml

File "test.ml", line 1, characters 8-9:
Parse error: [expr] expected after "-" (in [expr])
File "test.ml", line 1:
Error: Error while running external preprocessor
Command line: camlp4o 'test.ml' > /tmp/ocamlpp5879b9

test.cmo :
test.cmx :

echo $?

2

ocamldep -allow-approx -pp camlp4o test.ml

File "test.ml", line 2, characters 8-9:
Parse error: [expr] expected after "-" (in [expr])
File "test.ml", line 1:
Error: Error while running external preprocessor
Command line: camlp4o 'test.ml' > /tmp/ocamlpp40ff36

test.cmo : foo.cmo
test.cmx : foo.cmx

echo $?

0

I'm writing a patch to do the same with -ppx errors.

@vicuna
Copy link
Author

vicuna commented Dec 17, 2015

Comment author: @diml

One thing I'm wondering is whether we should fail for all uninterpreted extensions. If an extension isn't interpreted, then the result of ocamldep might be completely wrong anyway.

@vicuna
Copy link
Author

vicuna commented Dec 17, 2015

Comment author: @diml

Fixed in commit b95a642.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant