This is an archive of the discontinued LLVM Phabricator instance.

[libc++][NFC] Enable modernize-use-override
ClosedPublic

Authored by philnik on Apr 30 2022, 6:52 AM.

Details

Diff Detail

Event Timeline

philnik created this revision.Apr 30 2022, 6:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 30 2022, 6:52 AM
Herald added a subscriber: smeenai. · View Herald Transcript
philnik requested review of this revision.Apr 30 2022, 6:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 30 2022, 6:52 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Nice improvement! Looking at the UBSAN CI failures there are more overrides to be added.
Can you add an NFC to the title of the patch?

libcxx/include/__memory/shared_ptr.h
134

Just curious did you use -fix for clang-tidy?

libcxx/include/codecvt
216–228

Did the indention change?

248–259

Did the indention change?

283–294

Did the indention change?

317–328

Did the indention change?

351–362

Did the indention change?

385–396

Did the indention change? Please also validate similar changes.

philnik updated this revision to Diff 427099.May 4 2022, 11:47 AM
  • Add a few more overrides
philnik retitled this revision from [libc++] Enable modernize-use-override to [libc++][NFC] Enable modernize-use-override.Aug 23 2022, 4:52 PM
philnik updated this revision to Diff 455025.Aug 23 2022, 5:14 PM
philnik marked 7 inline comments as done.
  • Try to fix CI
libcxx/include/__memory/shared_ptr.h
134

Kind-of. I went trough the errors and let my IDE auto-fix them. Using -fix doesn't work very well because we copy the headers around.

libcxx/include/codecvt
216–228

The return type also doesn't have it's own line anymore. That's why the diff looks so weird.

248–259

It changed the same as the last one.

283–294

Same

317–328

same

351–362

same

385–396

same

Mordante accepted this revision.Aug 25 2022, 9:46 AM

LGTM!

This revision is now accepted and ready to land.Aug 25 2022, 9:46 AM
This revision was automatically updated to reflect the committed changes.