[APINotes] Support overloaded operators

[egorzhdan]

This adds support for annotating C++ operators via API Notes. For instance:

Tags:
- Name: MyTag
  Methods:
  - Name: operator+
    Availability: none

At the moment only operators that are declared as methods of C++ records can be annotated.

rdar://148534260

[llvmbot]

@llvm/pr-subscribers-clang

Author: Egor Zhdan (egorzhdan)

Changes

This adds support for annotating C++ operators via API Notes. For instance:

Tags:
- Name: MyTag
  Methods:
  - Name: operator+
    Availability: none

rdar://148534260

---

Full diff: https://github.com/llvm/llvm-project/pull/177382.diff

4 Files Affected:

• (modified) clang/lib/Sema/SemaAPINotes.cpp (+10-4)
• (modified) clang/test/APINotes/Inputs/Headers/Methods.apinotes (+6)
• (modified) clang/test/APINotes/Inputs/Headers/Methods.h (+2)
• (modified) clang/test/APINotes/methods.cpp (+10)

diff --git a/clang/lib/Sema/SemaAPINotes.cpp b/clang/lib/Sema/SemaAPINotes.cpp
index d041d0f1f0a30..059eb0dd767b7 100644
--- a/clang/lib/Sema/SemaAPINotes.cpp
+++ b/clang/lib/Sema/SemaAPINotes.cpp
@@ -1191,12 +1191,18 @@ void Sema::ProcessAPINotes(Decl *D) {
     if (auto CXXMethod = dyn_cast<CXXMethodDecl>(D)) {
       if (!isa<CXXConstructorDecl>(CXXMethod) &&
           !isa<CXXDestructorDecl>(CXXMethod) &&
-          !isa<CXXConversionDecl>(CXXMethod) &&
-          !CXXMethod->isOverloadedOperator()) {
+          !isa<CXXConversionDecl>(CXXMethod)) {
         for (auto Reader : APINotes.findAPINotes(D->getLocation())) {
           if (auto Context = UnwindTagContext(TagContext, APINotes)) {
-            auto Info =
-                Reader->lookupCXXMethod(Context->id, CXXMethod->getName());
+            std::string MethodName;
+            if (CXXMethod->isOverloadedOperator())
+              MethodName =
+                  std::string("operator") +
+                  getOperatorSpelling(CXXMethod->getOverloadedOperator());
+            else
+              MethodName = CXXMethod->getName();
+
+            auto Info = Reader->lookupCXXMethod(Context->id, MethodName);
             ProcessVersionedAPINotes(*this, CXXMethod, Info);
           }
         }
diff --git a/clang/test/APINotes/Inputs/Headers/Methods.apinotes b/clang/test/APINotes/Inputs/Headers/Methods.apinotes
index 98fb7c5a489cb..b9bc0a85b69b0 100644
--- a/clang/test/APINotes/Inputs/Headers/Methods.apinotes
+++ b/clang/test/APINotes/Inputs/Headers/Methods.apinotes
@@ -9,6 +9,12 @@ Tags:
   - Name: getDecremented
     Availability: none
     AvailabilityMsg: "this should have no effect"
+  - Name: operator+
+    Availability: none
+    AvailabilityMsg: "oh no, this is an operator"
+  - Name: operator*
+    Availability: none
+    AvailabilityMsg: "oh no, this is an operator star"
 - Name: IntWrapper2
   Methods:
   - Name: getIncremented
diff --git a/clang/test/APINotes/Inputs/Headers/Methods.h b/clang/test/APINotes/Inputs/Headers/Methods.h
index f4eb3589429d3..dbe28da521dd2 100644
--- a/clang/test/APINotes/Inputs/Headers/Methods.h
+++ b/clang/test/APINotes/Inputs/Headers/Methods.h
@@ -4,6 +4,8 @@ struct IntWrapper {
   IntWrapper getIncremented() const { return {value + 1}; }
 
   IntWrapper operator+(const IntWrapper& RHS) const { return {value + RHS.value}; }
+
+  const int& operator*() const { return value; }
 };
 
 extern "C++" {
diff --git a/clang/test/APINotes/methods.cpp b/clang/test/APINotes/methods.cpp
index a38642604a175..6e69c365d7ce6 100644
--- a/clang/test/APINotes/methods.cpp
+++ b/clang/test/APINotes/methods.cpp
@@ -1,6 +1,8 @@
 // RUN: rm -rf %t && mkdir -p %t
 // RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/Methods -fdisable-module-hash -fapinotes-modules -fsyntax-only -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -x c++
 // RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/Methods -fdisable-module-hash -fapinotes-modules -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -ast-dump -ast-dump-filter IntWrapper::getIncremented -x c++ | FileCheck --check-prefix=CHECK-METHOD %s
+// RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/Methods -fdisable-module-hash -fapinotes-modules -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -ast-dump -ast-dump-filter IntWrapper::operator+ -x c++ | FileCheck --check-prefix=CHECK-OPERATOR-PLUS %s
+// RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/Methods -fdisable-module-hash -fapinotes-modules -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -ast-dump -ast-dump-filter IntWrapper::operator* -x c++ | FileCheck --check-prefix=CHECK-OPERATOR-STAR %s
 // RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/Methods -fdisable-module-hash -fapinotes-modules -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -ast-dump -ast-dump-filter IntWrapper2::getIncremented -x c++ | FileCheck --check-prefix=CHECK-METHOD-2 %s
 // RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/Methods -fdisable-module-hash -fapinotes-modules -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -ast-dump -ast-dump-filter IntWrapper3::getIncremented -x c++ | FileCheck --check-prefix=CHECK-METHOD-3 %s
 // RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/Methods -fdisable-module-hash -fapinotes-modules -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -ast-dump -ast-dump-filter Outer::Inner::getDecremented -x c++ | FileCheck --check-prefix=CHECK-DEEP-METHOD %s
@@ -11,6 +13,14 @@
 // CHECK-METHOD-NEXT: CXXMethodDecl {{.+}} getIncremented
 // CHECK-METHOD: UnavailableAttr {{.+}} <<invalid sloc>> "oh no"
 
+// CHECK-OPERATOR-PLUS: Dumping IntWrapper::operator+:
+// CHECK-OPERATOR-PLUS-NEXT: CXXMethodDecl {{.+}} operator+
+// CHECK-OPERATOR-PLUS: UnavailableAttr {{.+}} <<invalid sloc>> "oh no, this is an operator"
+
+// CHECK-OPERATOR-STAR: Dumping IntWrapper::operator*:
+// CHECK-OPERATOR-STAR-NEXT: CXXMethodDecl {{.+}} operator*
+// CHECK-OPERATOR-STAR: UnavailableAttr {{.+}} <<invalid sloc>> "oh no, this is an operator star"
+
 // CHECK-METHOD-2: Dumping IntWrapper2::getIncremented:
 // CHECK-METHOD-2-NEXT: CXXMethodDecl {{.+}} getIncremented
 // CHECK-METHOD-2: UnavailableAttr {{.+}} <<invalid sloc>> "oh no"

[Xazax-hun]

The changes look good to me. I am a bit worried about the people misusing this though as operators in a namespace would apply to a potentially huge number of overloads.

[egorzhdan]

@Xazax-hun this only applies to operators that are declared as methods, so this wouldn't apply to operators in a namespace.

[compnerd]

Personally, I find that a ternary is easier to read:

Suggested change

	std::string MethodName;
	if (CXXMethod->isOverloadedOperator())
	MethodName =
	std::string("operator") +
	getOperatorSpelling(CXXMethod->getOverloadedOperator());
	else
	MethodName = CXXMethod->getName();
	std::string MethodName = CXXMethod->isOverloadedOperator()
	? std::string("operator") +
	getOperatorSpelling(CXXMethod->getOverloadedOperator())
	: CXXMethod->getName();

[egorzhdan]

I initially wrote something similar, but got confused about this expression after using clang-format, so I replaced it with if-else to make it slightly more clear.

[Xazax-hun]

this only applies to operators that are declared as methods,

This makes sense, thanks! Can you update the description of the PR with this info?

[egorzhdan]

Can you update the description of the PR with this info?

Done!