Missing warning when an attribute is ignored or makes no sense

[shindere]

For instance, if the [@@unboxed] attribute is applied to a type
for which unboxing makes no sense (because no unboxing is possible
for that type), no warning is available to let the user know that the
attribute is being ignored, so that the attribute can be ignored
silently.

[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).

[gasche]

As far as I can tell, we don't have an actual reproduction example for the original issue:

if the [@@unboxed] attribute is applied to a type
for which unboxing makes no sense (because no unboxing is possible
for that type), no warning is available to let the user know that the
attribute is being ignored

We discussed this in the online triaging meeting two weeks ago, and I mentioned that this issue is not actionable without an example. I will go ahead and close this. We can reopen if a concrete example shows up, or open a more precise/specific/informative issue. (For example @ccasin has a different regression in mind, explicitating an example might be useful. But he is planning to send a PR anyway so maybe this does not matter.)