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

silenced warnings from ours deprecations #885

Merged
merged 2 commits into from Oct 6, 2018

Conversation

gitoleg
Copy link
Contributor

@gitoleg gitoleg commented Oct 5, 2018

This PR silences warnings from bap code during compilation of bap itself. Also, it turned out that few modules in bap.mli were deprecated in a way that doesn't emit warnings at all.

Just for the record, the next sample doesn't deprecate module A, because according to the documentation for attributes of level 3:

They are not attached to any specific node in the syntax tree

module A = struct
  [@@@deprecated "[since ...]"]
  ...
end

And the next works as expected, although in this case, we have to do the depreciation at the end of the module, which makes it less readable for a user. But anyway, looks like we don't have any other choice.

module A = struct
  ...
end [@@deprecated "[since ...]"]

This PR silences warnings from bap code during compilation of
bap itself. Also, it turned out that few modules in `bap.mli`
were deprecated in a way that doesn't emit warnings at all:

Just for the record, the next doesn't deprecate `module A`,
because according to the documentation for attributes of level 3:
> They are not attached to any specific node in the syntax tree

```
module A = struct
  [@@@deprecated "[since ...]"]
  ...
end
```

And the next works as expected, although in this case we have
do the deprecation in the end of module, which makes it less
readable. But anyway, looks like we don't have any other choice.

```
module A = struct
  ...
end [@@deprecated "[since ...]"]
```
Copy link
Member

@ivg ivg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, but my main concern is that the deprecation attiribute is no longer at the top of the module and thus is not visible. So, we should ensure that we have at least @deprecated attribute in the documentation.

@gitoleg gitoleg merged commit c080b91 into BinaryAnalysisPlatform:master Oct 6, 2018
@gitoleg gitoleg deleted the resolves-warnings branch October 10, 2018 13:02
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

Successfully merging this pull request may close these issues.

None yet

2 participants