cross-platform macro to mark a function or class as deprecated

[Thiago Farina]

Hi,

I just saw in the book 'API Design for C++' (no, unfortunately I don't
have the book yet, just the source code) about a DEPRECATED macro.

The header looks like this:

deprecated.h
#ifdef __GNUC__
#define DEPRECATED __attribute__ ((deprecated))
#elif defined(_MSC_VER)
#define DEPRECATED __declspec(deprecated)
#else
#define DEPRECATED
#pragma message("DEPRECATED is not defined for this compiler")
#endif

Then we can tag our deprecated methods as:

DEPRECATED void DoSomething();

The code still compiles but you get a warning. This sounds very
similar to our OVERRIDE macro. Do you guys think it is worth having
it? Because today we do this through comments which is hard to
discover IMO.

--
Thiago

[Darin Fisher]

We compile with warnings treated as errors.  Won't that be a problem?  Deprecated methods are kept around until we have eliminated all callers.

On Oct 21, 2012 6:22 PM, "Thiago Farina" <tfa...@chromium.org> wrote:

[Thiago Farina]

On Sun, Oct 21, 2012 at 11:26 PM, Darin Fisher <da...@chromium.org> wrote:
> We compile with warnings treated as errors. Won't that be a problem?

If it's true, then yeah, it would be a problem.

> Deprecated methods are kept around until we have eliminated all callers.
>

I think some of them stay around because people don't know they are deprecated.

The warning looks like this:

../../chapter8/deprecate_compile_time/main.cc:7:5: warning: 'GetName'
is deprecated [-Wdeprecated-declarations]
f.GetName();
^
../../chapter8/deprecate_compile_time/foo.h:12:26: note: 'GetName' declared here
DEPRECATED std::string GetName();

It might not be all that worth though.

--
Thiago

[Ryan Sleevi]

On Sunday, October 21, 2012 6:26:29 PM UTC-7, Darin Fisher wrote:

We compile with warnings treated as errors.  Won't that be a problem?  Deprecated methods are kept around until we have eliminated all callers.

Yes, it would be a problem.

For that reason, you find littered throughout the Windows GYP files either _CRT_SECURE_NO_WARNINGS,  _CRT_SECURE_NO_DEPRECATE or _SCL_SECURE_NO_DEPRECATE, since a variety of POSIX/CRT/"standard" APIs have been marked deprecated in favour of Microsoft's Secure CRT functions (the value of those functions for 'security' being a subject of debate)

The solution to deprecation is to simply move fast. If you're going to deprecate something, implement the new method, convert the existing, and remove the deprecated method. Leaving deprecated methods in just adds to code cost, and even for "huge" refactorings, can often be done on an order of a week or two, and where the rate of code addition is far outpaced by the rate of refactoring.

[Adam Barth]

In WebKit, we've had some success putting the word "deprecated" in the
function name. That catches the eye of contributors and reviewer, who
know to look for a better alternative. (Of course, you actually need
to provide a better alternative or folks will end up using the
"deprecated" function anyway.)

Adam

[Ami Fischman]

FWIW, google3 used to use the attribute-based mechanism (for many years), and it turned out to be a bad idea.  Now we use a standardized comment form.  (google-internal-only link for the curious: go/mpfiu).

The main problem with the attribute is that it silently shifts the burden of work in a deprecation from the deprecator to the deprecatee, and in the worst (and sadly too-common) case, it means that the build becomes much spammier, with developers becoming inured to new/further spam (therefore detracting from the original point of adding such a warning!).

Arguments can be made many ways (the benefit is worth the cost, the cost I describe above is unlikely, and so on) but I'm offering the above experience as an anecdote about what can happen in a very large codebase touched by many developers with unaligned immediate goals.  I believe adding the attribute to the chromium codebase is the wrong thing to do.  I like abarth@'s suggestion of putting Deprecated into the symbol's name.

Cheers,

-a

[Joao da Silva]

src/PRESUBMIT.py has a list of banned functions that will trigger presubmit warnings or errors when added in new CLs (look for _BANNED_CPP_FUNCTIONS in the file). I don't know if using this is encouraged, but it catches misuses of deprecated functions and makes the CQ fail.

- Joao

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Marc-Antoine Ruel]

Be aware of edge cases that could block devs in unexpected ways. For example these kinds of presubmit makes it near impossible to move some files with the CQ. This can be a real problem for non-committers.

One way to reduce the interference is to make the deprecation check a warning on upload but a notification on commit.

Just sayin, I don't really mind.

M-A