C++: Mark deprecated overrides as deprecated

[dbartol]

Fixes github/codeql-c-analysis-team#79

The QL compiler is about to be changed to emit a warning when overriding a deprecated predicate. This PR marks the existing overrides of deprecated predicates as deprecated themselves, which avoids the warning.

The Print.qll models seem to preserve the isWideCharDefault() predicate for backwards compatibility, so we can't remove them and must continue overriding them.

The XML.qll override is necessary because both superclasses declare the getName() predicate. One is deprecated, and the other is abstract, so we have to have an override.

[rdmarsh2]

The XML.qll override is necessary because both superclasses declare the getName() predicate. One is deprecated, and the other is abstract, so we have to have an override.

Does this mean that uses of XMLParent::getName() will not trigger deprecation warnings, but uses of XMLFile::getName() will? Also, with XML.qll being synced between languages, will this cause XMLFile::getName() to be deprecated for languages for which File::getName() doesn't exist?

[rdmarsh2]

Actually, File::getName() has been deprecated for over two years, along with a lot of other File methods. I think we can just delete them instead.

[dbartol]

Ugh. Good points. @geoffw0 @jbj, can I just remove the long-deprecated File::getName() and avoid this whole mess?

[aschackmull]

Ugh. Good points. @geoffw0 @jbj, can I just remove the long-deprecated File::getName() and avoid this whole mess?

That's what I did for Java in #3507

[jbj]

👍 This sounds like a great time to remove those long-deprecated member predicates on File.

[geoffw0]

These changes are simpler than I'd expecteded.

isWideCharDefault changes LGTM.

I'm not so happy with deprecating XMLFile.getName(). If it disambiguates between deprecated and a non-deprecated parent class predicates I would expect it to inherit non-deprecated status. Seems like a bug that this is being flagged?

I'm happy with removing File.getName() as a solution, but this does trigger a chain reaction of removing other predicates from File.qll. Perhaps that's not a bad thing.

[ginsbach]

These changes are simpler than I'd expecteded.

isWideCharDefault changes LGTM.

I'm not so happy with deprecating XMLFile.getName(). If it disambiguates between deprecated and a non-deprecated parent class predicates I would expect it to inherit non-deprecated status. Seems like a bug that this is being flagged?

I'm happy with removing File.getName() as a solution, but this does trigger a chain reaction of removing other predicates from File.qll. Perhaps that's not a bad thing.

Thanks for the suggestion. I have discussed this with my team and changed the PR to only report the warning when all overridden members are deprecated.

[dbartol]

I've undone the change to XML.qll.
I removed all of the deprecated predicates from File that were not themselves overrides of other deprecated predicates. That was enough to fix the new warnings and remove some cruft, without turning into a multi-day unravelling of the C++ library.

[dbartol]

@geoffw0 Is it OK to merge, then?

[geoffw0]

LGTM.