This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Make readability-container-size-empty check using <string> header
ClosedPublic

Authored by mikecrowe on Mar 4 2023, 1:11 PM.

Details

Summary

Improve the generic <string> header by adding the size() method so that
it can be used to replace the custom implementation in the
readability-container-size-empty check.

This requires fixing an incorrect comparison of a std::wstring with a
char string literal.

Unfortunately, removing the custom basic_string implementation means
fixing the line numbers for many of the checks.

Depends on D145312

Diff Detail

Event Timeline

mikecrowe created this revision.Mar 4 2023, 1:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 4 2023, 1:11 PM
Herald added a subscriber: xazax.hun. · View Herald Transcript
mikecrowe requested review of this revision.Mar 4 2023, 1:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 4 2023, 1:11 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Could you upload the patch with full context? I believe you need to do something like git show HEAD -U999999 as per the guidelines. Otherwise arc diff should do the job automatically. Reason I ask is that I cannot see the new line numbers referred to by the note comments - Phab says "Context not available".

Could you upload the patch with full context? I believe you need to do something like git show HEAD -U999999 as per the guidelines. Otherwise arc diff should do the job automatically. Reason I ask is that I cannot see the new line numbers referred to by the note comments - Phab says "Context not available".

Of course. It had been sufficiently long since I last uploaded a new diff that I forgot that part of the instructions and just used git format-patch for this batch. I shall try to learn how to use arc diff because the copying and pasting of diffs was getting rather tedious and I knew I was bound to make a mistake with it eventually.

Thanks!

mikecrowe updated this revision to Diff 503896.Mar 9 2023, 12:54 PM

Upload using arc diff so more context is available.

Now a descendent of D145724.

Now a descendent of D145724.

Oh. The commit message update didn't make it through. :( Never mind, this doesn't really depend on D145724 since that change modified a different part of <string>.

PiotrZSL accepted this revision.Mar 10 2023, 1:55 AM

LGTM, this is anyway non functional change

This revision is now accepted and ready to land.Mar 10 2023, 1:55 AM