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

Version 4.2 fails to compile unused attributes #148

Open
pdonadeo opened this issue Aug 28, 2017 · 18 comments
Open

Version 4.2 fails to compile unused attributes #148

pdonadeo opened this issue Aug 28, 2017 · 18 comments

Comments

@pdonadeo
Copy link

I have this type definition:

type raw_region_group = {
  name : string;
  iitc_draw_inline : (raw_iitc_draw option [@default None]);
  iitc_draw_filename : (string option [@default None]);
  json_tiles_inline : (string list option [@default None]);
  json_tiles_filename : (string option [@default None]);
  text_tiles_filename : (string option [@default None]);
  accounts : IntelBot.account list;
} [@@deriving sexp, yojson]

It works with version 4.1 but fails in 4.2 with this error:

File "src/configuration.ml", line 11, characters 45-52:
Error: Attribute `default' was not used.
Hint: `default' is available for label declarations but is used here in the
context of a core type.
Did you put it at the wrong level?

default is actually used by yojson but not by sexp. Is this behaviour the expected one? This actually prevents to mix two generators.

@whitequark
Copy link
Collaborator

ppx_deriving was designed with namespacing in mind; use [@yojson.default None] everywhere to work around issues like this.

@pdonadeo
Copy link
Author

@whitequark thanks for your suggestion but something is going wrong:

File "src/configuration.ml", line 11, characters 45-59:
Error: Attribute `yojson.default' was not used

Any ideas? Maybe the culprit is not ppx_deriving but the specific filter, yojson?

@mjambon FYI

In this moment I worked around downgrading ppx_deriving to 4.1.

@whitequark
Copy link
Collaborator

@pdonadeo Try [@deriving.yojson.default].

@pdonadeo
Copy link
Author

@whitequark nope, same error as before 😅

File "src/configuration.ml", line 11, characters 45-68:
Error: Attribute `deriving.yojson.default' was not used

@whitequark
Copy link
Collaborator

Hmm, this might be a ppx_type_conv issue... I'll look at it

@pdonadeo
Copy link
Author

Thanks @whitequark. It's not urgent for me, I can happily live with version 4.1 for a while. Do you need a minimal code that stimulate this issue?

@whitequark
Copy link
Collaborator

That would be helpful

@pdonadeo
Copy link
Author

New OPAM switch:

$ opam switch --alias-of=4.04.1 4.04.1-ISSUE-148
$ opam install core ppx_deriving ppx_deriving_yojson

main.ml:

open Core


type person = {
  first_name : string;
  last_name : string;
  age : (int option [@deriving.yojson.default None]);
} [@@deriving sexp, yojson]

let _ =
  Printf.printf "Hello!\n%!"

Compile with:

ocamlbuild -use-ocamlfind \
    -cflag -thread -lflag -thread \
    -package core -package ppx_deriving_yojson -package ppx_sexp_conv \
    main.native

The error is:

File "main.ml", line 7, characters 22-45:
Error: Attribute `deriving.yojson.default' was not used
Command exited with code 2.
Compilation unsuccessful after building 1 target (0 cached) in 00:00:00.

Downgrading compiles again:

$ opam pin add ppx_deriving 4.1
$ ocamlfind ocamlc -thread -i -package ppx_sexp_conv -package ppx_deriving_yojson -package core main.ml > main.inferred.mli
$ cat main.inferred.mli
type person = { first_name : string; last_name : string; age : int option; }
val person_of_sexp : Sexplib.Sexp.t -> person
val sexp_of_person : person -> Sexplib.Sexp.t
val person_to_yojson : person -> Yojson.Safe.json
val person_of_yojson :
  Yojson.Safe.json -> person Ppx_deriving_yojson_runtime.error_or

@emillon
Copy link
Contributor

emillon commented Aug 31, 2017

I confirm that this happens also with [@compare] and [@equal].

It seems that it's also triggering errors when other extensions like ppx_blob are used in the same file. ("Extension `blob' was not translated").

@xclerc
Copy link

xclerc commented Sep 27, 2017

The error is actually raised by ppx_core that is used by ppx_sexp_conv.
Indeed, ppx_core does some sanity checks and reports unused attributes,
not knowing that another non-ppx_core-based deriver will use them later on.

With @diml, we discussed the possibility to change these errors into warnings
plugged into the AST (i.e. [@@ocaml.ppwarning "some message"]), and
have a patch ready. However, it does "silence" the issue but does not really
solve it.

@copy
Copy link
Contributor

copy commented Nov 10, 2017

Temporary workaroud until ppx_core is fixed: Add -no-check to the command line of ppx_sexp_conv or ppx_jane.

@xclerc
Copy link

xclerc commented Nov 10, 2017

The latest versions of ppx_driver (0.9.2) disables the checks by default.
They can be enabled through a new command-line switch, namely -do-check.

@tomjridge
Copy link

tomjridge commented Nov 14, 2017

I also have this problem. As a non-ppx expert, how do I add -no-check to my Makefile-based build?

@copy
Copy link
Contributor

copy commented Nov 14, 2017

I also have this problem. As a non-ppx expert, how do I add -no-check to my Makefile-based build?

Find the -pp or -ppx flags and add -no-check after each preprocessor. It should look something like this:

ocamlfind ocamlopt … -pp 'ppx-jane -no-check'

@gasche
Copy link
Contributor

gasche commented Nov 23, 2017

Do I correctly understand that marking ppx_deriving 4.2.x as conflicting with ppx_driver < 0.9.2 would make people's life easier by ruling this bug out?

@emillon
Copy link
Contributor

emillon commented Nov 23, 2017

Probably, but on the other hand it seems impossible to install ppx_driver.0.9.2 on 4.04.2 or 4.05.0, so this would restrict a lot the potential users of ppx_deriving.4.2.

@gasche
Copy link
Contributor

gasche commented Nov 23, 2017

@xclerc would you consider releasing 4.04.2 or 4.05.0 versions of ppx_driver with the disabled check?

@xclerc
Copy link

xclerc commented Nov 23, 2017

ppx_driver only constraints available: [ ocaml-version >= "4.03.0" ];
the problem comes from ppx_ast that, in its v0.9.2 version, uses a function
that does not exist (or rather has a different name) before 4.06.

(I still have to check whether it is the only problem.)

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

No branches or pull requests

7 participants