This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Fix readability-static-accessed-through-instance check for anonymous structs
ClosedPublic

Authored by AMS21 on Apr 2 2023, 4:08 AM.

Details

Summary

Previously we would provide a fixit which looked like
this unnamed struct at ...::f() but which is obviously
not valid C/C++.

Since there is no real nice way to accesses a static function
from an anonymous struct anyways we simply ignore all
anonymous structs.

Fixes llvm#61736

Diff Detail

Event Timeline

AMS21 created this revision.Apr 2 2023, 4:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 2 2023, 4:08 AM
AMS21 requested review of this revision.Apr 2 2023, 4:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 2 2023, 4:08 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
AMS21 added inline comments.Apr 2 2023, 4:11 AM
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
79

I wonder if there's a cleaner and more performant way to check this.
I've tried isAnonymousStructOrUnion() from the RecordDecl but that seems to be about something else.

carlosgalvezp added inline comments.Apr 2 2023, 8:24 AM
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
79

Indeed this seems brittle. Since RecordDecl inherits from NamedDecl, have you tried the getName() function? Hopefully it's an empty string and we can use that instead?

carlosgalvezp added inline comments.Apr 2 2023, 8:42 AM
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
79

Actually this in NamedDecl sounds like what you could use here?

  /// Get the identifier that names this declaration, if there is one.
  ///
  /// This will return NULL if this declaration has no name (e.g., for
  /// an unnamed class) or if the name is a special name (C++ constructor,
  /// Objective-C selector, etc.).
  IdentifierInfo *getIdentifier() const { return Name.getAsIdentifierInfo(); }
``
AMS21 updated this revision to Diff 510537.Apr 3 2023, 9:39 AM
AMS21 marked 2 inline comments as done.

Implement suggested way of checking for anonymous structs
Add a new more tests

PiotrZSL accepted this revision.EditedApr 3 2023, 10:08 AM

LGTM, will you commit this or want someone else to commit this ?

This revision is now accepted and ready to land.Apr 3 2023, 10:08 AM
AMS21 added a comment.Apr 3 2023, 11:13 PM

LGTM, will you commit this or want someone else to commit this ?

As I don't have access to the repository. I need someone else to commit it.