This is a first step in the right direction, but I wasn't able to get an
exhaustive list of all deprecated components per standard, so there's
certainly stuff that's missing.
rdar://problem/18168350
Differential D48912
[libc++] Add deprecated attributes to many deprecated components ldionne on Jul 3 2018, 6:51 PM. Authored by
Details This is a first step in the right direction, but I wasn't able to get an rdar://problem/18168350
Diff Detail
Event TimelineComment Actions We'll also want tests that very the deprecated diagnostics are emitted correctly. To test for the production of diagnostics, Libc++ uses *.fail.cpp tests along with Clang Verify.
Comment Actions Like the "don't discard return values" patch that is currently under review (D45179), this needs to be opt-in - otherwise people will complain (loudly). Comment Actions So if someone defines (say) _LIBCXX_DEPRECATION_WARNINGS in their build flags, they get the warnings. Otherwise, not.
Comment Actions Addressed reviewer feedback.
Comment Actions I'm not sure this is a good/correct example. Comment Actions FWIW, I would prefer to enable deprecation warnings by default and to have a way of turning them off -- so as to be as close to the standard as possible by default. But I'm fine either way, and this is always a choice we can revisit later by just switching the default from opt-in to opt-out.
Comment Actions I have a strong preference for this as well.
I think that silencing deprecation warnings by default does users a disservice. Comment Actions Regarding whether we want to enable or disable those warnings by default: it seems like we've reached consensus (on the LLVM IRC) for enabling the deprecation warnings by default. However, I suggest we keep them disabled by default in this commit, and then change the default in a subsequent change. This will make things easier from an integration perspective, and also in case we need to roll back the defaults because of some unforeseen circumstances.
Comment Actions Made sure the attributes worked properly on GCC. This still requires using attribute((deprecated)) whenever that's Comment Actions In general this lgtm. First comment I think you need to fix, others can go together in a follow-up I think.
Comment Actions I still think this is good to go as-is, given my answer to JF's comments.
|