Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006493OCamlOCaml generalpublic2014-07-19 14:442014-08-05 16:59
Reporterwhitequark 
Assigned Tofrisch 
PrioritynormalSeverityminorReproducibilityhave not tried
StatusresolvedResolutionwon't fix 
PlatformOSOS Version
Product Version4.02.0+beta1 / +rc1 
Target VersionFixed in Version 
Summary0006493: Warn on uninterpreted annotations
DescriptionI believe there should be a compiler warning for uninterpreted (i.e. not removed from the AST at typecheck time and not belonging to ocaml. namespace) annotations.

I will motivate with an example. Ppx_protobuf, my type-driven code generation library that automatically derives Protobuf encoders/decoders, uses field attributes, e.g. [@encoding], to specify an option that drastically affects the generated code and therefore the public API of the application. It should be specified on the core_type:

    type packet = { length : int [@encoding uint] }

However, for every correct placement of an annotation, there are innumerably more incorrect placements; or space for mistyping an annotation name:
    
    type packet = { length [@encoding uint] : int }
    type packet = { length : int [@encdoing uint] }

It is not practical to check every single attribute array in the AST (how would you test this in a sane way?), and checking for typos is simply impossible in presence of multiple unconstrained AST rewriters.

However, I think it is incredibly important for usability of ppx rewriters that misplaced attributes or attributes with typos are not simply ignored.

Therefore, I propose that:

  * An AST rewriter would remove the attributes it knows, and only those, from the AST upon completion,
  * OCaml compiler would gain a warning emitted when an attribute left in the AST is encountered,
  * this warning is enabled by default.
TagsNo tags attached.
Attached Files

- Relationships

-  Notes
(0011888)
lpw25 (developer)
2014-07-19 16:33
edited on: 2014-07-19 16:34

Attributes are not solely for the use of ppx. They flow through the compiler into ".cmt" and ".cmi" files. This can be useful for a variety of tools (e.g. documentation generators, code analyses). Warning on uninterpreted attributes would remove this important aspect of attributes.

You mention that it is difficult to check that all uses of a particular attribute have been found. However, it should be quite easy to provide support for this through the Ast_mapper interface. There are probably other things we could do to improve error checking of attributes, but since they are very extensible it is always going to be difficult. Where possible extension nodes should be used instead, as these will give an error if not interpreted by a ppx.

(0011890)
frisch (developer)
2014-07-21 10:11

As Leo said, attributes are not only intended for ppx mappers. Other uses include tools that parses .cmt/.cmti/.cmi files, tools which re-parse the source code, and the compiler itself (including some specific backends, and future versions of the compiler -- if we even introduce an "inline" attribute on function definitions, we don't want older versions of the compiler to complain on such attributes). Moreover, even ppx mappers don't necessarily want to remove attributes they understand: the same attributes could be used by other tools as well (e.g. a "default" attribute on a record field could be used to feed several "data-binding" code generators), and it can make sense to leave attributes so that they appear in .cmt/.cmti/.cmi files, e.g. for documentation generators (which count as "another tool").

So the proposal for a warning against attributes flowing to the compiler is overly restrictive.

That said, it could be interesting to formalize at least the supported locations (probably including some nesting information, how they interact with extension nodes, and maybe the form of supported payloads) for attributes, for each tool, using an ad hoc language. Such formal descriptions could be fed to a dedicated "type checker for attributes" tool (either a standalone tool or an identity ppx). This would support the use of attributes not only by ppx mappers, but also those tools which parse .cmt/.cmti/.cmi files, and those who re-parse the sources.

I can imagine that each tool would come with its description of supported attributes.

Those formal descriptions could also make it possible to detect automatically potential conflicts between attributes used by several tools in incompatible ways. They could also support IDEs and editor modes.

We probably need to wait to see more precisely how attributes will be used to design such as type system for attributes.
If such a nice system emerges, gets used widely by the community and is simple enough, it could even be integrated in the compiler directly (and probably executed before ppx mappers).

Anyway, I'm marking this issue as "won't fix", but I encourage everyone to contribute to a nice type system for attributes!
(0011891)
whitequark (developer)
2014-07-21 12:22

Alain, you have very good points and I completely agree that your proposed solution is much better.

- Issue History
Date Modified Username Field Change
2014-07-19 14:44 whitequark New Issue
2014-07-19 16:33 lpw25 Note Added: 0011888
2014-07-19 16:34 lpw25 Note Edited: 0011888 View Revisions
2014-07-21 10:11 frisch Note Added: 0011890
2014-07-21 10:11 frisch Status new => resolved
2014-07-21 10:11 frisch Resolution open => won't fix
2014-07-21 10:11 frisch Assigned To => frisch
2014-07-21 12:22 whitequark Note Added: 0011891


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker