This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] support unscoped enumerations in readability-static-accessed-through-instance
ClosedPublic

Authored by HerrCai0907 on Mar 31 2023, 4:15 AM.

Details

Summary

fixed 60810
unscoped enumerations in class can also be checked by readability-static-accessed-through-instance
add matcher for enumConstantDecl to match format

struct {
    enum { E1 };
};

The filter of member expression and the fix hint should be same as other condition.

Diff Detail

Event Timeline

HerrCai0907 created this revision.Mar 31 2023, 4:15 AM
HerrCai0907 requested review of this revision.Mar 31 2023, 4:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2023, 4:15 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Add test with scoped enums, to validate that it works correctly.
In theory this change should suport them also.

And update commit message to be more descriptive...

Eugene.Zelenko added inline comments.
clang-tools-extra/docs/ReleaseNotes.rst
278

Please keep alphabetical order (by check name) in this section.

LegalizeAdulthood resigned from this revision.Mar 31 2023, 8:47 AM

Add test with scoped enums, to validate that it works correctly.
In theory this change should suport them also.

scoped enum not support to visit by member. EnumName:: is needed for scoped enum. And ins.EnumName is invalid.

PiotrZSL requested changes to this revision.Mar 31 2023, 3:41 PM

Ok, you right.
Fix all open issues, and update commit message to be more detailed.
Except that, LGTM.

This revision now requires changes to proceed.Mar 31 2023, 3:41 PM

update description

Well, description update didn't update, you can update it here in UI, just edit revision, if you use "arc", there is a switch to apply description update.

Would it make sense to add a scoped enum to the test, to ensure it doesn't warn/fix?

HerrCai0907 edited the summary of this revision. (Show Details)Apr 1 2023, 6:05 PM
HerrCai0907 edited the summary of this revision. (Show Details)

Would it make sense to add a scoped enum to the test, to ensure it doesn't warn/fix?

It is not possible to visit scoped enum by instance. I think adding an invalid expression to test it is meaningless.

PiotrZSL accepted this revision.Apr 2 2023, 8:43 AM
This revision is now accepted and ready to land.Apr 2 2023, 8:43 AM