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

Add a deprecated warning #5854

Closed
vicuna opened this issue Dec 14, 2012 · 9 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link

commented Dec 14, 2012

Original bug ID: 5854
Reporter: @ppedrot
Assigned to: @alainfrisch
Status: closed (set by @xavierleroy on 2015-12-11T18:27:51Z)
Resolution: fixed
Priority: normal
Severity: feature
Fixed in version: 4.02.0+dev
Category: ~DO NOT USE (was: OCaml general)
Tags: patch
Related to: #6160
Monitored by: jmeber @hcarty

Bug description

It would be nice to be able to flag functions as deprecated directly in the source code, so that ocamlc/opt would spit out a warning when using such a function.

This would ease code maintenance a lot.

File attachments

@vicuna

This comment has been minimized.

Copy link
Author

commented Dec 14, 2012

Comment author: @gasche

I guess it is currently possible to do that by inspection of the .cmt file spit out by the -bin-annot files: traverse AST in the .cmt and check for uses of deprecated functions; to be integrated in your compilation&test toolchain. To declare a function deprecated you could either just give the traverser-tool a list of deprecated functions, or use a hack to embed the annotation as valid OCaml source code (not what I would choose):

deprecate.mli
type 'a t = 'a

in your code:
let f = ... in
ignore (f : _ Deprecate.t);

I'm not sure we want to integrate this ad-hoc feature directly in the language itself -- I'd rather wait until someone comes with a decent pragmas/annotations proposal. In the meantime I think a bin-annot-based tool could suit your needs.

@vicuna

This comment has been minimized.

Copy link
Author

commented Jun 28, 2013

Comment author: @damiendoligez

This will certainly become easy to do when the extension_points feature gets integrated.

@vicuna

This comment has been minimized.

Copy link
Author

commented Sep 18, 2013

Comment author: @alainfrisch

Now that extension_points has been integrated, I've uploaded a patch against trunk which triggers warning 3 (Deprecated feature) when we refer to a value whose declaration (in a signature) has been marked with the [@@deprecated] attribute.

To make the information easily accessible to the type-checker (without forcing it to read .cmti files), the patch keeps the attributes in the Types.value_description records (and value descriptions coming from implementation, not interface, currently don't generate any attribute) which are stored in particular in .cmi files. After this patch, detecting a reference to a value marked as deprecated is indeed trivial.

Gabriel suggests a different approach, based on an external tool reading both .cmt files (to find value references) and .cmti files (to find value declarations). Unfortunately, finding the value declaration in a .cmti file corresponding to a value reference in a .cmt file is not absolutely straightforward: this amounts to redoing part of the symbol resolution strategy done by the compiler, which is not trivial. Consider for instance the case where the .mli file has an "include" statement: the .cmti file will contain only a symbolic Tsig_include item, and to find the real value declaration, we need to follow the link.

If we start giving some special meaning to a few attributes in the compiler itself, it makes sense, in my opinion, to keep attributes in .cmi files as well. Would anyone object to it? This is also related to the work on runtime types: it would be natural to keep (some) attributes on type declarations in the runtime description of types. This would also probably require to keep them in .cmi files.

(We could also question the relation between .cmi and .cmti files and maybe somehow merge them.)

@vicuna

This comment has been minimized.

Copy link
Author

commented Sep 18, 2013

Comment author: @gasche

Unfortunately, finding the value declaration in a .cmti file
corresponding to a value reference in a .cmt file is not absolutely
straightforward: this amounts to redoing part of the symbol
resolution strategy done by the compiler, which is not
trivial. Consider for instance the case where the .mli file has an
"include" statement: the .cmti file will contain only a symbolic
Tsig_include item, and to find the real value declaration, we need
to follow the link.

Could we provide a facility to do just that in compiler-libs? I think
we have a blind spot around "referring to declarations" that we're
hitting from several different sides (this was also one of the major
aspects of the "integrate location information into
.cmi" discussion) -- and that has to be redone by ocamlspotter,
Merlin, ocp-index, etc..

(Do tell if you think it's a good idea but don't have the time or will
do implement it. We may ask the developers of those projects if
they've something that we could integrate.)

@vicuna

This comment has been minimized.

Copy link
Author

commented Sep 20, 2013

Comment author: @alainfrisch

Could we provide a facility to do just that in compiler-libs?

This would certainly be useful (but indeed I don't plan to work on it). That being said, I don't think that the compiler should read the .cmt/.cmti files and redo some lookup in them; some features deserve to be incorporated in the compiler directly (warnings on unused or deprecated stuff, etc). But yes, external tools would need similar resolution logic and it would be useful to provide it somewhere.

@vicuna

This comment has been minimized.

Copy link
Author

commented Sep 26, 2013

Comment author: @alainfrisch

The patch has been integrated (commit 14188 in trunk).

@vicuna

This comment has been minimized.

Copy link
Author

commented Sep 27, 2013

Comment author: @alainfrisch

Commit 14191: also detect references to types or constructors marked as deprecated.

@vicuna

This comment has been minimized.

Copy link
Author

commented May 25, 2014

Comment author: @damiendoligez

Please note: the name of the attribute was changed to [@@ocaml.deprecated].

@vicuna

This comment has been minimized.

Copy link
Author

commented May 25, 2014

Comment author: @alainfrisch

Actually, both deprecated and ocaml.deprecated are accepted (and similarly for all built-in attributes).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.