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

Mark deprecated functionality with attributes and remove usage of such features #1022

Merged
merged 3 commits into from
Feb 2, 2019
Merged

Mark deprecated functionality with attributes and remove usage of such features #1022

merged 3 commits into from
Feb 2, 2019

Conversation

eliaskosunen
Copy link
Contributor

@eliaskosunen eliaskosunen commented Jan 29, 2019

This PR marks features commented as deprecated with attributes; [[deprecated]] if compiled with C++14 and __attribute__((deprecated)) or __declspec(deprecated) if not.

I was able to replace most of the cases where deprecated functionality was used in the library, but one case lingers: https://github.com/eliaskosunen/fmt/blob/599632ee226d74d6c34d89290ebabd7320c90afe/include/fmt/format.h#L3061

arg_map::init takes const basic_format_args<Context>& as its sole argument, which can only be extracted from context_base with the deprecated member function args().

@eliaskosunen
Copy link
Contributor Author

Travis builds are erroring because of -Werror and the usage of deprecated context_base::args().

@vitaut
Copy link
Contributor

vitaut commented Jan 30, 2019

This is great, thanks!

arg_map::init takes const basic_format_args& as its sole argument, which can only be extracted from context_base with the deprecated member function args().

I suggest making args protected and adding a using declaration in basic_format_context for backward compatibility. Not sure if using declaration can be deprecated.

@eliaskosunen
Copy link
Contributor Author

eliaskosunen commented Feb 1, 2019

[[deprecated]] cannot be used with using-declarations, to that option is off the table. I circumvented the issue by simply commenting the FMT_DEPRECATED out and adding a comment explaining it. I couldn't come up with any alternative ways of fixing the issue apart from some messy ones requiring way too much work. I think this should be fixed at some later time.

There was also some usage of the deprecated format_decimal overload in format-test.cc. For that, I simply disabled the warning with pragmas. I don't know that part of the library all that well, so I'm not really one to fix it.

@vitaut vitaut merged commit 7fbbfed into fmtlib:master Feb 2, 2019
@vitaut
Copy link
Contributor

vitaut commented Feb 2, 2019

Merged, thanks!

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.

2 participants