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

[AMS21]

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

[AMS21]

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]

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]

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]

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

[PiotrZSL]

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

[AMS21]

In D147411#4241016, @PiotrZSL wrote:

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.

clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp

if (MemberExpression->getBeginLoc().isMacroID())
     return;
 
   const Expr *BaseExpr = MemberExpression->getBase();
 
   // Do not warn for overloaded -> operators.
   if (isa<CXXOperatorCallExpr>(BaseExpr))
     return;
 
-  QualType BaseType =
+  const QualType BaseType =
       BaseExpr->getType()->isPointerType()
           ? BaseExpr->getType()->getPointeeType().getUnqualifiedType()
           : BaseExpr->getType().getUnqualifiedType();
 
   const ASTContext *AstContext = Result.Context;
   PrintingPolicy PrintingPolicyWithSuppressedTag(AstContext->getLangOpts());
   PrintingPolicyWithSuppressedTag.SuppressTagKeyword = true;
   PrintingPolicyWithSuppressedTag.SuppressUnwrittenScope = true;
 
   PrintingPolicyWithSuppressedTag.PrintCanonicalTypes =
       !BaseExpr->getType()->isTypedefNameType();
 
   std::string BaseTypeName =
       BaseType.getAsString(PrintingPolicyWithSuppressedTag);
 
+  // Ignore anonymous structs/classes which will not have an identifier
+  const RecordDecl *RecDecl = BaseType->getAsCXXRecordDecl();
+  if (!RecDecl || RecDecl->getIdentifier() == nullptr)
+    return;
+
   // Do not warn for CUDA built-in variables.
   if (StringRef(BaseTypeName).startswith("__cuda_builtin_"))
     return;
 
   SourceLocation MemberExprStartLoc = MemberExpression->getBeginLoc();
   auto Diag =
       diag(MemberExprStartLoc, "static member accessed through instance");
 

clang-tools-extra/docs/ReleaseNotes.rst

- Improved :doc:`readability-magic-numbers
   magic numbers in type aliases such as ``using`` and ``typedef`` declarations if
   the new ``IgnoreTypeAliases`` option is set to true.
 
 - Fixed a false positive in :doc:`readability-misleading-indentation
   <clang-tidy/checks/readability/misleading-indentation>` check when warning would
   be unnecessarily emitted for template dependent ``if constexpr``.
 
 - Improved :doc:`readability-static-accessed-through-instance
-  <clang-tidy/checks/readability/static-accessed-through-instance>` check to 
-  support unscoped enumerations through instances.
+  <clang-tidy/checks/readability/static-accessed-through-instance>` check to
+  support unscoped enumerations through instances and fixed usage of anonymous
+  structs or classes.
 
 - Fixed a false positive in :doc:`cppcoreguidelines-slicing
   <clang-tidy/checks/cppcoreguidelines/slicing>` check when warning would be
   emitted in constructor for virtual base class initialization.
 
 - Improved :doc:`bugprone-use-after-move
   <clang-tidy/checks/bugprone/use-after-move>` to understand that there is a
   sequence point between designated initializers.

clang-tools-extra/test/clang-tidy/checkers/readability/static-accessed-through-instance.cpp

 unsigned int x2 = blockIdx.x;
 // CHECK-MESSAGES-NOT: :[[@LINE-1]]:10: warning: static member
 unsigned int x3 = blockDim.x;
 // CHECK-MESSAGES-NOT: :[[@LINE-1]]:10: warning: static member
 unsigned int x4 = gridDim.x;
 // CHECK-MESSAGES-NOT: :[[@LINE-1]]:10: warning: static member
 
 } // namespace Bugzilla_48758
+
+// https://github.com/llvm/llvm-project/issues/61736
+namespace llvm_issue_61736
+{
+
+struct {
+  static void f() {}
+} AnonStruct, *AnonStructPointer;
+
+class {
+  public:
+  static void f() {}
+} AnonClass, *AnonClassPointer;
+
+void testAnonymousStructAndClass() {
+  AnonStruct.f();
+  AnonStructPointer->f();
+
+  AnonClass.f();
+  AnonClassPointer->f();
+}
+
+struct Embedded {
+  struct {
+    static void f() {}
+  } static EmbeddedStruct, *EmbeddedStructPointer;
+
+  class {
+    public:
+      static void f() {}
+  } static EmbeddedClass, *EmbeddedClassPointer;
+};
+
+void testEmbeddedAnonymousStructAndClass() {
+  Embedded::EmbeddedStruct.f();
+  Embedded::EmbeddedStructPointer->f();
+
+  Embedded::EmbeddedClass.f();
+  Embedded::EmbeddedClassPointer->f();
+
+  Embedded E;
+  E.EmbeddedStruct.f();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member accessed through instance [readability-static-accessed-through-instance]
+  // CHECK-FIXES: {{^}}  llvm_issue_61736::Embedded::EmbeddedStruct.f();{{$}}
+  E.EmbeddedStructPointer->f();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member accessed through instance [readability-static-accessed-through-instance]
+  // CHECK-FIXES: {{^}}  llvm_issue_61736::Embedded::EmbeddedStructPointer->f();{{$}}
+
+  E.EmbeddedClass.f();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member accessed through instance [readability-static-accessed-through-instance]
+  // CHECK-FIXES: {{^}}  llvm_issue_61736::Embedded::EmbeddedClass.f();{{$}}
+  E.EmbeddedClassPointer->f();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member accessed through instance [readability-static-accessed-through-instance]
+  // CHECK-FIXES: {{^}}  llvm_issue_61736::Embedded::EmbeddedClassPointer->f();{{$}}
+}
+
+} // namespace llvm_issue_61736