Missing warning when an attribute is ignored or makes no sense

[shindere]

Missing warning when an attribute is ignored or makes no sense

[gasche]

In general it is a known usability issue that the compiler has an "open world" approach to attributes: any attribute we see may have been meant for a preprocessor instead of us, so we ignore them silently instead of assuming that they are incorrect -- even when they are typos, or misplaced compiler-supported attributes. The Jane Street fork has improvements in this direction, relying on assumptions about the preprocessor (they use the ppxlib strategy of removing the attributes they handle): ocaml-flambda/ocaml-jst#44 . I believe that they ( @goldfirere, @ccasin ) would be interested in help upstreaming this work.

In the specific case that you mention (adding [@@unboxed] to a type that does not support unboxing), there should be an error. Do you have an example?

[Octachron]

This work has already upstreamed in #12451: we don't have an open world approach for builtin compiler attributes anymore.

Instead, we record all known compilers attributes during parsing, and we mark them whenever they are used. The misplaced warning is then emitted if there were compiler attributes without consumers. This is not 100% exact yet, because for some attributes (mostly alerts) it is not that clear to decide when an attribute is consumed.

@shindere , do you have a precise example of missing warning for an unused attribute?

A possibility that is occuring to me now is that the reworked misplaced-attribute warning 57 doesn't trigger in the toplevel. Did you test in a toplevel?

[ccasin]

I do think there is a bug where the warning doesn't trigger in the top level. We just noticed this internally last week. I meant to take a look at fixing, which is probably just one line, but have been swamped - soon!

[gasche]

Oof, sorry for forgetting about this!

[shindere]

Chris Casinghino (2024/05/09 04:14 -0700):

I do think there is a bug where the warning doesn't trigger in the top level. We just noticed this internally last week. I meant to take a look at fixing, which is probably just one line, but have been swamped - soon!

thanks a lot! If you provide a fix, will you please also add a test tomake sure the fix does work and prevent regressions?

[tmcgilchrist]

# Using unboxed attribute on an Effect
$ echo 'type _ Effect.t += Foo : int -> int Effect.t [@@unboxed]' > test.ml
$ opam exec --switch="5.2.0" -- ocamlc -c test.ml
File "test.ml", line 1, characters 48-55:
1 | type _ Effect.t += Foo : int -> int Effect.t [@@unboxed]
                                                    ^^^^^^^
Warning 53 [misplaced-attribute]: the "unboxed" attribute cannot appear in this context
$ opam exec --switch="5.1.1" -- ocamlc -c test.ml 
# No Warning

# Using spelling mistake unbxed
$ echo 'type _ Effect.t += Foo : int -> int Effect.t [@@unbxed]' > test_2.ml
$ opam exec --switch="5.2.0" -- ocamlc -c test_2.ml
# No warning
$ opam exec --switch="5.1.1" -- ocamlc -c test_2.ml
# No warning

[ccasin]

thanks a lot! If you provide a fix, will you please also add a test tomake sure the fix does work and prevent regressions?

Indeed - I'll be sure to add a test. I have a local version of the fix, but it will need to wait until after #13170 (because there are many attributes in tests that incorrectly give warning 53 once I fix this issue, if that one isn't fixed first).

[ccasin]

@tmcgilchrist Both of your examples look expected to me. [@unboxed] does not work on extensible variants, and warning 53 cannot detect spelling mistakes (because the attribute may be intended for an external tool like a ppx - we can only warn about attributes known to the compiler but in the wrong place).