[libclang] Expose arguments of clang::annotate

[fridtjof]

Summary

This enables easy consumption of arbitrary data added
to this annotation in addition to the annotation category,
which was already exposed.

[aaron.ballman]

Thank you for the additions! This should have test coverage in clang/unitttests/libclang/LibclangTest.cpp and a release note.

clang/tools/libclang/CIndex.cpp

2755

We could use a template type and enable_if (for some type safety), then it shares the implementation.

[fridtjof]

• Deduplicated code by using templates (looks a bit ridiculous though)
• Release notes entry added
• Unit test added

[aaron.ballman]

I think precommit CI may have found a relevant failure:

******************** TEST 'Clang :: Index/IBOutletCollection.m' FAILED ********************
Script:
--
: 'RUN: at line 8';   c:\ws\w5\llvm-project\premerge-checks\build\bin\c-index-test.exe -cursor-at=C:\ws\w5\llvm-project\premerge-checks\clang\test\Index\IBOutletCollection.m:4:24 C:\ws\w5\llvm-project\premerge-checks\clang\test\Index\IBOutletCollection.m | c:\ws\w5\llvm-project\premerge-checks\build\bin\filecheck.exe -check-prefix=CHECK-CURSOR C:\ws\w5\llvm-project\premerge-checks\clang\test\Index\IBOutletCollection.m
: 'RUN: at line 11';   c:\ws\w5\llvm-project\premerge-checks\build\bin\c-index-test.exe -test-annotate-tokens=C:\ws\w5\llvm-project\premerge-checks\clang\test\Index\IBOutletCollection.m:4:1:5:1 C:\ws\w5\llvm-project\premerge-checks\clang\test\Index\IBOutletCollection.m | c:\ws\w5\llvm-project\premerge-checks\build\bin\filecheck.exe -check-prefix=CHECK-TOK C:\ws\w5\llvm-project\premerge-checks\clang\test\Index\IBOutletCollection.m
--
Exit Code: 1
 
Command Output (stdout):
--
$ ":" "RUN: at line 8"
$ "c:\ws\w5\llvm-project\premerge-checks\build\bin\c-index-test.exe" "-cursor-at=C:\ws\w5\llvm-project\premerge-checks\clang\test\Index\IBOutletCollection.m:4:24" "C:\ws\w5\llvm-project\premerge-checks\clang\test\Index\IBOutletCollection.m"
$ "c:\ws\w5\llvm-project\premerge-checks\build\bin\filecheck.exe" "-check-prefix=CHECK-CURSOR" "C:\ws\w5\llvm-project\premerge-checks\clang\test\Index\IBOutletCollection.m"
# command stderr:
C:\ws\w5\llvm-project\premerge-checks\clang\test\Index\IBOutletCollection.m:9:18: error: CHECK-CURSOR: expected string not found in input
// CHECK-CURSOR: ObjCClassRef=Test:3:12
                 ^
<stdin>:1:1: note: scanning from here
4:3 attribute(iboutletcollection)= [IBOutletCollection=ObjCInterface] Extent=[4:3 - 4:27]
^
<stdin>:1:54: note: possible intended match here
4:3 attribute(iboutletcollection)= [IBOutletCollection=ObjCInterface] Extent=[4:3 - 4:27]
                                                     ^
 
Input file: <stdin>
Check file: C:\ws\w5\llvm-project\premerge-checks\clang\test\Index\IBOutletCollection.m
 
-dump-input=help explains the following input dump.
 
Input was:
<<<<<<
           1: 4:3 attribute(iboutletcollection)= [IBOutletCollection=ObjCInterface] Extent=[4:3 - 4:27]
check:9'0     X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
check:9'1                                                          ?                                     possible intended match
>>>>>>
 
error: command failed with exit status: 1
 
--
 
********************

clang/unittests/libclang/LibclangTest.cpp

1176

Spurious comment?

1178

We don't seem to be testing this?

[fridtjof]

I fixed the failing IBOutletCollection.m test - there was code to do special handling for the respective attribute type, which my patch accidentally skipped previously. I reordered the generic attribute handling to come after the special cases in VisitChildren.

I cleaned up my unit test. After trying a bit to fix it for testing annotate_type as well, I found out that for some unknown reason it is not exposed like usual attributes (maybe because it's a type attribute?).
Therefore, I decided to drop it from the test altogether.

[aaron.ballman]

In D151373#4533550, @fridtjof wrote:

I fixed the failing IBOutletCollection.m test - there was code to do special handling for the respective attribute type, which my patch accidentally skipped previously. I reordered the generic attribute handling to come after the special cases in VisitChildren.

Nice!

I cleaned up my unit test. After trying a bit to fix it for testing annotate_type as well, I found out that for some unknown reason it is not exposed like usual attributes (maybe because it's a type attribute?).
Therefore, I decided to drop it from the test altogether.

I think we should drop the annotate_type functionality from the patch entirely rather than add it without any test coverage knowing it doesn't seem to work as expected.

[fridtjof]

In D151373#4538604, @aaron.ballman wrote:

I think we should drop the annotate_type functionality from the patch entirely rather than add it without any test coverage knowing it doesn't seem to work as expected.

Agreed - I removed the superfluous code and also cleaned up the summary accordingly.

[fridtjof]

Whoops, forgot to run clang-format on the last diff. Also took the opportunity to just get rid of the template mess now that we're only dealing with AnnotateAttr anyway.

[aaron.ballman]

LGTM! Do you need me to commit this on your behalf? If so, what name and email address would you like me to use for patch attribution?

[fridtjof]

(Whoops, somehow never got a mail for that reply)

Yes, please. You can attribute the commit to "fridtjof <[e-mail]>"

[vitalybuka]

clang/tools/libclang/CIndex.cpp

3662

There is a leak here https://lab.llvm.org/buildbot/#/builders/5/builds/35725

[vitalybuka]

Not sure why the similar code above does not leak, so I will revert and leave to author to investigate

[vitalybuka]

clang/tools/libclang/CIndex.cpp

3662

Please ignore the location of this comment.
I don't know which part of the patch is responsible for the leak
Another trace here https://lab.llvm.org/buildbot/#/builders/236/builds/5562/steps/13/logs/stdio

[fridtjof]

It was the unit test I added - I forgot to properly dispose of the EvalResult there

[aaron.ballman]

In D151373#4578560, @fridtjof wrote:

It was the unit test I added - I forgot to properly dispose of the EvalResult there

I've re-landed the changes in 177ec17944af5fb87445017c3ac9028bbc1d710f, thank you!

clang/docs/ReleaseNotes.rst

 ------------
 
 clang-format
 ------------
 
 libclang
 --------
 
+- Exposed arguments of ``clang::annotate``.
+
 Static Analyzer
 ---------------
 
 .. _release-notes-sanitizers:
 
 Sanitizers
 ----------
 

clang/tools/libclang/CIndex.cpp

 #include "CXFile.h"
 #include "CXSourceLocation.h"
 #include "CXString.h"
 #include "CXTranslationUnit.h"
 #include "CXType.h"
 #include "CursorVisitor.h"
 #include "clang-c/FatalErrorHandler.h"
 #include "clang/AST/Attr.h"
+#include "clang/AST/AttrVisitor.h"
 #include "clang/AST/DeclObjCCommon.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/Mangle.h"
 #include "clang/AST/OpenMPClause.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/Diagnostic.h"
if (Cursor.kind == CXCursor_IBOutletCollectionAttr) {
     const IBOutletCollectionAttr *A =
         cast<IBOutletCollectionAttr>(cxcursor::getCursorAttr(Cursor));
     if (const ObjCObjectType *ObjT = A->getInterface()->getAs<ObjCObjectType>())
       return Visit(cxcursor::MakeCursorObjCClassRef(
           ObjT->getInterface(),
           A->getInterfaceLoc()->getTypeLoc().getBeginLoc(), TU));
   }
 
+  if (clang_isAttribute(Cursor.kind)) {
+    if (const Attr *A = getCursorAttr(Cursor))
+      return Visit(A);
+
+    return false;
+  }
+
   // If pointing inside a macro definition, check if the token is an identifier
   // that was ever defined as a macro. In such a case, create a "pseudo" macro
   // expansion cursor for that token.
   SourceLocation BeginLoc = RegionOfInterest.getBegin();
   if (Cursor.kind == CXCursor_MacroDefinition &&
       BeginLoc == RegionOfInterest.getEnd()) {
     SourceLocation Loc = AU->mapLocationToPreamble(BeginLoc);
     const MacroInfo *MI =
public:
   const FieldDecl *get() const {
     return static_cast<const FieldDecl *>(data[0]);
   }
   SourceLocation getLoc() const {
     return SourceLocation::getFromRawEncoding(
         (SourceLocation::UIntTy)(uintptr_t)data[1]);
   }
 };
-class EnqueueVisitor : public ConstStmtVisitor<EnqueueVisitor, void> {
+class EnqueueVisitor : public ConstStmtVisitor<EnqueueVisitor, void>,
+                       public ConstAttrVisitor<EnqueueVisitor, void> {
   friend class OMPClauseEnqueue;
   VisitorWorkList &WL;
   CXCursor Parent;
 
 public:
   EnqueueVisitor(VisitorWorkList &wl, CXCursor parent)
       : WL(wl), Parent(parent) {}
 
void VisitOMPTargetTeamsDistributeDirective(
       const OMPTargetTeamsDistributeDirective *D);
   void VisitOMPTargetTeamsDistributeParallelForDirective(
       const OMPTargetTeamsDistributeParallelForDirective *D);
   void VisitOMPTargetTeamsDistributeParallelForSimdDirective(
       const OMPTargetTeamsDistributeParallelForSimdDirective *D);
   void VisitOMPTargetTeamsDistributeSimdDirective(
       const OMPTargetTeamsDistributeSimdDirective *D);
 
+  // Attributes
+  void VisitAnnotateAttr(const AnnotateAttr *A);
+
 private:
   void AddDeclarationNameInfo(const Stmt *S);
   void AddNestedNameSpecifierLoc(NestedNameSpecifierLoc Qualifier);
   void AddExplicitTemplateArgs(const TemplateArgumentLoc *A,
                                unsigned NumTemplateArgs);
   void AddMemberRef(const FieldDecl *D, SourceLocation L);
   void AddStmt(const Stmt *S);
   void AddDecl(const Decl *D, bool isFirst = true);
   void AddTypeLoc(TypeSourceInfo *TI);
   void EnqueueChildren(const Stmt *S);
   void EnqueueChildren(const OMPClause *S);
+  template <typename AnnAttr,
+            std::enable_if_t<std::is_same<AnnAttr, AnnotateAttr>::value,
+                             bool> = true>
+  void EnqueueChildren(const AnnAttr *A);
 };
 } // namespace
 
 void EnqueueVisitor::AddDeclarationNameInfo(const Stmt *S) {
   // 'S' should always be non-null, since it comes from the
   // statement we are visiting.
   WL.push_back(DeclarationNameInfoVisit(S, Parent));
 }
void EnqueueVisitor::EnqueueChildren(const OMPClause *S) {
   Visitor.Visit(S);
   if (size == WL.size())
     return;
   // Now reverse the entries we just added.  This will match the DFS
   // ordering performed by the worklist.
   VisitorWorkList::iterator I = WL.begin() + size, E = WL.end();
   std::reverse(I, E);
 }
+
+template <typename AnnAttr,
+          std::enable_if_t<std::is_same<AnnAttr, AnnotateAttr>::value,
+                           bool>>
+void EnqueueVisitor::EnqueueChildren(const AnnAttr *A) {
+  unsigned size = WL.size();
+  for (const Expr *Arg : A->args()) {
+    VisitStmt(Arg);
+  }
+  if (size == WL.size())
+    return;
+  // Now reverse the entries we just added.  This will match the DFS
+  // ordering performed by the worklist.
+  VisitorWorkList::iterator I = WL.begin() + size, E = WL.end();
+  std::reverse(I, E);
+}
+
 void EnqueueVisitor::VisitAddrLabelExpr(const AddrLabelExpr *E) {
   WL.push_back(LabelRefVisit(E->getLabel(), E->getLabelLoc(), Parent));
 }
 void EnqueueVisitor::VisitBlockExpr(const BlockExpr *B) {
   AddDecl(B->getBlockDecl());
 }
 void EnqueueVisitor::VisitCompoundLiteralExpr(const CompoundLiteralExpr *E) {
   EnqueueChildren(E);

 }
 void EnqueueVisitor::VisitSizeOfPackExpr(const SizeOfPackExpr *E) {
   WL.push_back(SizeOfPackExprParts(E, Parent));
 }
 void EnqueueVisitor::VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
   // If the opaque value has a source expression, just transparently
   // visit that.  This is useful for (e.g.) pseudo-object expressions.
   if (Expr *SourceExpr = E->getSourceExpr())
-    return Visit(SourceExpr);
+    return ConstStmtVisitor::Visit(SourceExpr);
 }
 void EnqueueVisitor::VisitLambdaExpr(const LambdaExpr *E) {
   AddStmt(E->getBody());
   WL.push_back(LambdaExprParts(E, Parent));
 }
 void EnqueueVisitor::VisitConceptSpecializationExpr(
     const ConceptSpecializationExpr *E) {
   WL.push_back(ConceptSpecializationExprVisit(E, Parent));
 }
 void EnqueueVisitor::VisitRequiresExpr(const RequiresExpr *E) {
   WL.push_back(RequiresExprVisit(E, Parent));
   for (ParmVarDecl *VD : E->getLocalParameters())
     AddDecl(VD);
 }
 void EnqueueVisitor::VisitCXXParenListInitExpr(const CXXParenListInitExpr *E) {
   EnqueueChildren(E);
 }
 void EnqueueVisitor::VisitPseudoObjectExpr(const PseudoObjectExpr *E) {
   // Treat the expression like its syntactic form.
-  Visit(E->getSyntacticForm());
+  ConstStmtVisitor::Visit(E->getSyntacticForm());
 }
 
 void EnqueueVisitor::VisitOMPExecutableDirective(
     const OMPExecutableDirective *D) {
   EnqueueChildren(D);
   for (ArrayRef<OMPClause *>::iterator I = D->clauses().begin(),
                                        E = D->clauses().end();
        I != E; ++I)
void EnqueueVisitor::VisitOMPTargetTeamsDistributeParallelForSimdDirective(
   VisitOMPLoopDirective(D);
 }
 
 void EnqueueVisitor::VisitOMPTargetTeamsDistributeSimdDirective(
     const OMPTargetTeamsDistributeSimdDirective *D) {
   VisitOMPLoopDirective(D);
 }
 
+void EnqueueVisitor::VisitAnnotateAttr(const AnnotateAttr *A) {
+  EnqueueChildren(A);
+}
+
 void CursorVisitor::EnqueueWorkList(VisitorWorkList &WL, const Stmt *S) {
   EnqueueVisitor(WL, MakeCXCursor(S, StmtParent, TU, RegionOfInterest))
-      .Visit(S);
+      .ConstStmtVisitor::Visit(S);
+}
+
+void CursorVisitor::EnqueueWorkList(VisitorWorkList &WL, const Attr *A) {
+  // Parent is the attribute itself when this is indirectly called from
+  // VisitChildren. Because we need to make a CXCursor for A, we need *its*
+  // parent.
+  auto AttrCursor = Parent;
+
+  // Get the attribute's parent as stored in
+  // cxcursor::MakeCXCursor(const Attr *A, const Decl *Parent, CXTranslationUnit
+  // TU)
+  const Decl *AttrParent = static_cast<const Decl *>(AttrCursor.data[1]);
+
+  EnqueueVisitor(WL, MakeCXCursor(A, AttrParent, TU))
+      .ConstAttrVisitor::Visit(A);
 }
 
 bool CursorVisitor::IsInRegionOfInterest(CXCursor C) {
   if (RegionOfInterest.isValid()) {
     SourceRange Range = getRawCursorExtent(C);
     if (Range.isInvalid() || CompareRegionOfInterest(Range))
       return false;
   }
if (!WorkListFreeList.empty()) {
     WorkListCache.push_back(WL);
   }
   EnqueueWorkList(*WL, S);
   bool result = RunVisitorWorkList(*WL);
   WorkListFreeList.push_back(WL);
   return result;
 }
 
+bool CursorVisitor::Visit(const Attr *A) {
+  VisitorWorkList *WL = nullptr;
+  if (!WorkListFreeList.empty()) {
+    WL = WorkListFreeList.back();
+    WL->clear();
+    WorkListFreeList.pop_back();
+  } else {
+    WL = new VisitorWorkList();
+    WorkListCache.push_back(WL);
+  }
+  EnqueueWorkList(*WL, A);
+  bool result = RunVisitorWorkList(*WL);
+  WorkListFreeList.push_back(WL);
+  return result;
+}
+
 namespace {
 typedef SmallVector<SourceRange, 4> RefNamePieces;
 RefNamePieces buildPieces(unsigned NameFlags, bool IsMemberRefExpr,
                           const DeclarationNameInfo &NI, SourceRange QLoc,
                           const SourceRange *TemplateArgsLoc = nullptr) {
   const bool WantQualifier = NameFlags & CXNameRange_WantQualifier;
   const bool WantTemplateArgs = NameFlags & CXNameRange_WantTemplateArgs;
   const bool WantSinglePiece = NameFlags & CXNameRange_WantSinglePiece;

clang/tools/libclang/CursorVisitor.h

#include "clang/AST/TypeLocNodes.def"
   bool VisitTagTypeLoc(TagTypeLoc TL);
   bool VisitArrayTypeLoc(ArrayTypeLoc TL);
   bool VisitFunctionTypeLoc(FunctionTypeLoc TL, bool SkipResultType = false);
 
   // Data-recursive visitor functions.
   bool IsInRegionOfInterest(CXCursor C);
   bool RunVisitorWorkList(VisitorWorkList &WL);
   void EnqueueWorkList(VisitorWorkList &WL, const Stmt *S);
+  void EnqueueWorkList(VisitorWorkList &WL, const Attr *A);
   LLVM_ATTRIBUTE_NOINLINE bool Visit(const Stmt *S);
+  LLVM_ATTRIBUTE_NOINLINE bool Visit(const Attr *A);
 
 private:
   std::optional<bool> handleDeclForVisitation(const Decl *D);
 };
 
 } // namespace cxcursor
 } // namespace clang
 
 #endif

clang/unittests/libclang/LibclangTest.cpp

if (cursor.kind == CXCursor_UnaryOperator) {
       return CXChildVisit_Break;
     }
 
     return CXChildVisit_Recurse;
   });
 }
 
 TEST_F(LibclangParseTest, VisitStaticAssertDecl_noMessage) {
   const char testSource[] = R"cpp(static_assert(true))cpp";
   std::string fileName = "main.cpp";
   WriteFile(fileName, testSource);
   const char *Args[] = {"-xc++"};
   ClangTU = clang_parseTranslationUnit(Index, fileName.c_str(), Args, 1,
                                        nullptr, 0, TUFlags);
 
   std::optional<CXCursor> staticAssertCsr;
   Traverse([&](CXCursor cursor, CXCursor parent) -> CXChildVisitResult {
     if (cursor.kind == CXCursor_StaticAssert) {
       staticAssertCsr.emplace(cursor);
Traverse(*staticAssertCsr, [&argCnt](CXCursor cursor, CXCursor parent) {
     }
     ++argCnt;
     return CXChildVisit_Continue;
   });
   ASSERT_EQ(argCnt, 2);
   EXPECT_EQ(fromCXString(clang_getCursorSpelling(*staticAssertCsr)), "");
 }
 
+TEST_F(LibclangParseTest, ExposesAnnotateArgs) {
+  const char testSource[] = R"cpp(
+[[clang::annotate("category", 42)]]
+void func() {}
+)cpp";
+  std::string fileName = "main.cpp";
+  WriteFile(fileName, testSource);
+
+  const char *Args[] = {"-xc++"};
+  ClangTU = clang_parseTranslationUnit(Index, fileName.c_str(), Args, 1,
+                                       nullptr, 0, TUFlags);
+
+  int attrCount = 0;
+
+  Traverse(
+      [&attrCount](CXCursor cursor, CXCursor parent) -> CXChildVisitResult {
+        if (cursor.kind == CXCursor_AnnotateAttr) {
+          int childCount = 0;
+          clang_visitChildren(
+              cursor,
+              [](CXCursor child, CXCursor,
+                 CXClientData data) -> CXChildVisitResult {
+                int *pcount = static_cast<int *>(data);
+
+                // we only expect one argument here, so bail otherwise
+                EXPECT_EQ(*pcount, 0);
+
+                auto *result = clang_Cursor_Evaluate(child);
+                EXPECT_NE(result, nullptr);
+                EXPECT_EQ(clang_EvalResult_getAsInt(result), 42);
+                ++*pcount;
+
+                return CXChildVisit_Recurse;
+              },
+              &childCount);
+          attrCount++;
+          return CXChildVisit_Continue;
+        }
+        return CXChildVisit_Recurse;
+      });
+
+  EXPECT_EQ(attrCount, 1);
+}
+
 class LibclangRewriteTest : public LibclangParseTest {
 public:
   CXRewriter Rew = nullptr;
   std::string Filename;
   CXFile File = nullptr;
 
   void SetUp() override {
     LibclangParseTest::SetUp();