This is an archive of the discontinued LLVM Phabricator instance.

Use 'override/final' instead of 'virtual' for overridden methods
ClosedPublic

Authored by alexfh on Apr 9 2015, 10:26 AM.

Details

Reviewers
dblaikie
Summary

The patch is generated using clang-tidy misc-use-override check.

This command was used:

tools/clang/tools/extra/clang-tidy/tool/run-clang-tidy.py -checks='-*,misc-use-override' -header-filter='llvm|clang' -j=32 -fix

Diff Detail

Repository
rL LLVM

Event Timeline

alexfh updated this revision to Diff 23500.Apr 9 2015, 10:26 AM
alexfh retitled this revision from to Use 'override/final' instead of 'virtual' for overridden methods.
alexfh updated this object.
alexfh edited the test plan for this revision. (Show Details)
alexfh added a reviewer: dblaikie.
alexfh set the repository for this revision to rL LLVM.
alexfh added a subscriber: Unknown Object (MLST).

A lot of these seem to be destructors. Do we really want override on
destructors? It's sort of implicit when the class isn't a base class.

A lot of these seem to be destructors. Do we really want override on
destructors? It's sort of implicit when the class isn't a base class.

Destructors are not much different from other methods in this regard: when you forget to specify virtual on the base class destructor, override on the derived one's will make compiler complain. Without override the code will compile fine, but will do something wrong.

alexfh updated this revision to Diff 23505.Apr 9 2015, 10:39 AM

svn diff | clang-format-diff -i

alexfh updated this revision to Diff 23508.Apr 9 2015, 10:42 AM

oops, wrong patch name supplied to arc, reverting to the original patch.

alexfh updated this revision to Diff 23514.Apr 9 2015, 11:12 AM
alexfh set the repository for this revision to rL LLVM.

Recreated the patch using:

tools/clang/tools/extra/clang-tidy/tool/run-clang-tidy.py -checks='-*,misc-use-override' -header-filter='llvm|clang' -j=32 -fix -format

For the record, this is awesome and we should totally do this.

-eric

dblaikie accepted this revision.Apr 10 2015, 4:37 PM
dblaikie edited edge metadata.

Sounds good to me.

This shouldn't really hurt blame any more than the existing transformations Ben did to add 'override' in the first place, so I don't see any disadvantage to this.

This revision is now accepted and ready to land.Apr 10 2015, 4:37 PM
alexfh closed this revision.Apr 10 2015, 7:15 PM

Committed revision 234679.