Remove duplicate API

[Jugst3r]

And adapt the existing code to account for the comments made when introducing the duplicate API.

Note that this introduces a retro-incompatibility with LLVM 19.

cc @sebastianpoeplau

[llvmbot]

@llvm/pr-subscribers-clang

Author: None (Jugst3r)

Changes

And adapt the existing code to account for the comments made when introducing the duplicate API.

Note that this introduces a retro-incompatibility with LLVM 19.

cc @sebastianpoeplau

---

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

5 Files Affected:

• (modified) clang/bindings/python/clang/cindex.py (+4-2)
• (modified) clang/include/clang-c/Index.h (+35-87)
• (modified) clang/tools/c-index-test/c-index-test.c (+3-3)
• (modified) clang/tools/libclang/CIndex.cpp (+6-31)
• (modified) clang/tools/libclang/libclang.map (-6)

diff --git a/clang/bindings/python/clang/cindex.py b/clang/bindings/python/clang/cindex.py
index e881bf131d6c4..afb5eb2af2821 100644
--- a/clang/bindings/python/clang/cindex.py
+++ b/clang/bindings/python/clang/cindex.py
@@ -1876,7 +1876,9 @@ def binary_operator(self):
         """
 
         if not hasattr(self, "_binopcode"):
-            self._binopcode = conf.lib.clang_Cursor_getBinaryOpcode(self)
+            self._binopcode = (
+                conf.lib.clang_Cursor_getCursorBinaryOperatorKind(self)
+            )
 
         return BinaryOperator.from_id(self._binopcode)
 
@@ -4043,7 +4045,7 @@ def set_property(self, property, value):
     ("clang_Cursor_getTemplateArgumentType", [Cursor, c_uint], Type),
     ("clang_Cursor_getTemplateArgumentValue", [Cursor, c_uint], c_longlong),
     ("clang_Cursor_getTemplateArgumentUnsignedValue", [Cursor, c_uint], c_ulonglong),
-    ("clang_Cursor_getBinaryOpcode", [Cursor], c_int),
+    ("clang_Cursor_getCursorBinaryOperatorKind", [Cursor], c_int),
     ("clang_Cursor_getBriefCommentText", [Cursor], _CXString),
     ("clang_Cursor_getRawCommentText", [Cursor], _CXString),
     ("clang_Cursor_getOffsetOfField", [Cursor], c_longlong),
diff --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h
index 38e2417dcd181..dbb49d181d684 100644
--- a/clang/include/clang-c/Index.h
+++ b/clang/include/clang-c/Index.h
@@ -3839,59 +3839,6 @@ enum CX_StorageClass {
   CX_SC_Register
 };
 
-/**
- * Represents a specific kind of binary operator which can appear at a cursor.
- */
-enum CX_BinaryOperatorKind {
-  CX_BO_Invalid = 0,
-  CX_BO_PtrMemD = 1,
-  CX_BO_PtrMemI = 2,
-  CX_BO_Mul = 3,
-  CX_BO_Div = 4,
-  CX_BO_Rem = 5,
-  CX_BO_Add = 6,
-  CX_BO_Sub = 7,
-  CX_BO_Shl = 8,
-  CX_BO_Shr = 9,
-  CX_BO_Cmp = 10,
-  CX_BO_LT = 11,
-  CX_BO_GT = 12,
-  CX_BO_LE = 13,
-  CX_BO_GE = 14,
-  CX_BO_EQ = 15,
-  CX_BO_NE = 16,
-  CX_BO_And = 17,
-  CX_BO_Xor = 18,
-  CX_BO_Or = 19,
-  CX_BO_LAnd = 20,
-  CX_BO_LOr = 21,
-  CX_BO_Assign = 22,
-  CX_BO_MulAssign = 23,
-  CX_BO_DivAssign = 24,
-  CX_BO_RemAssign = 25,
-  CX_BO_AddAssign = 26,
-  CX_BO_SubAssign = 27,
-  CX_BO_ShlAssign = 28,
-  CX_BO_ShrAssign = 29,
-  CX_BO_AndAssign = 30,
-  CX_BO_XorAssign = 31,
-  CX_BO_OrAssign = 32,
-  CX_BO_Comma = 33,
-  CX_BO_LAST = CX_BO_Comma
-};
-
-/**
- * \brief Returns the operator code for the binary operator.
- */
-CINDEX_LINKAGE enum CX_BinaryOperatorKind
-clang_Cursor_getBinaryOpcode(CXCursor C);
-
-/**
- * \brief Returns a string containing the spelling of the binary operator.
- */
-CINDEX_LINKAGE CXString
-clang_Cursor_getBinaryOpcodeStr(enum CX_BinaryOperatorKind Op);
-
 /**
  * Returns the storage class for a function or variable declaration.
  *
@@ -6671,73 +6618,74 @@ CINDEX_LINKAGE unsigned clang_visitCXXMethods(CXType T, CXFieldVisitor visitor,
  */
 enum CXBinaryOperatorKind {
   /** This value describes cursors which are not binary operators. */
-  CXBinaryOperator_Invalid,
+  CXBinaryOperator_Invalid = 0,
   /** C++ Pointer - to - member operator. */
-  CXBinaryOperator_PtrMemD,
+  CXBinaryOperator_PtrMemD = 1,
   /** C++ Pointer - to - member operator. */
-  CXBinaryOperator_PtrMemI,
+  CXBinaryOperator_PtrMemI = 2,
   /** Multiplication operator. */
-  CXBinaryOperator_Mul,
+  CXBinaryOperator_Mul = 3,
   /** Division operator. */
-  CXBinaryOperator_Div,
+  CXBinaryOperator_Div = 4,
   /** Remainder operator. */
-  CXBinaryOperator_Rem,
+  CXBinaryOperator_Rem = 5,
   /** Addition operator. */
-  CXBinaryOperator_Add,
+  CXBinaryOperator_Add = 6,
   /** Subtraction operator. */
-  CXBinaryOperator_Sub,
+  CXBinaryOperator_Sub = 7,
   /** Bitwise shift left operator. */
-  CXBinaryOperator_Shl,
+  CXBinaryOperator_Shl = 8,
   /** Bitwise shift right operator. */
-  CXBinaryOperator_Shr,
+  CXBinaryOperator_Shr = 9,
   /** C++ three-way comparison (spaceship) operator. */
-  CXBinaryOperator_Cmp,
+  CXBinaryOperator_Cmp = 10,
   /** Less than operator. */
-  CXBinaryOperator_LT,
+  CXBinaryOperator_LT = 11,
   /** Greater than operator. */
-  CXBinaryOperator_GT,
+  CXBinaryOperator_GT = 12,
   /** Less or equal operator. */
-  CXBinaryOperator_LE,
+  CXBinaryOperator_LE = 13,
   /** Greater or equal operator. */
-  CXBinaryOperator_GE,
+  CXBinaryOperator_GE = 14,
   /** Equal operator. */
-  CXBinaryOperator_EQ,
+  CXBinaryOperator_EQ = 15,
   /** Not equal operator. */
-  CXBinaryOperator_NE,
+  CXBinaryOperator_NE = 16,
   /** Bitwise AND operator. */
-  CXBinaryOperator_And,
+  CXBinaryOperator_And = 17,
   /** Bitwise XOR operator. */
-  CXBinaryOperator_Xor,
+  CXBinaryOperator_Xor = 18,
   /** Bitwise OR operator. */
-  CXBinaryOperator_Or,
+  CXBinaryOperator_Or = 19,
   /** Logical AND operator. */
-  CXBinaryOperator_LAnd,
+  CXBinaryOperator_LAnd = 20,
   /** Logical OR operator. */
-  CXBinaryOperator_LOr,
+  CXBinaryOperator_LOr = 21,
   /** Assignment operator. */
-  CXBinaryOperator_Assign,
+  CXBinaryOperator_Assign = 22,
   /** Multiplication assignment operator. */
-  CXBinaryOperator_MulAssign,
+  CXBinaryOperator_MulAssign = 23,
   /** Division assignment operator. */
-  CXBinaryOperator_DivAssign,
+  CXBinaryOperator_DivAssign = 24,
   /** Remainder assignment operator. */
-  CXBinaryOperator_RemAssign,
+  CXBinaryOperator_RemAssign = 25,
   /** Addition assignment operator. */
-  CXBinaryOperator_AddAssign,
+  CXBinaryOperator_AddAssign = 26,
   /** Subtraction assignment operator. */
-  CXBinaryOperator_SubAssign,
+  CXBinaryOperator_SubAssign = 27,
   /** Bitwise shift left assignment operator. */
-  CXBinaryOperator_ShlAssign,
+  CXBinaryOperator_ShlAssign = 28,
   /** Bitwise shift right assignment operator. */
-  CXBinaryOperator_ShrAssign,
+  CXBinaryOperator_ShrAssign = 29,
   /** Bitwise AND assignment operator. */
-  CXBinaryOperator_AndAssign,
+  CXBinaryOperator_AndAssign = 30,
   /** Bitwise XOR assignment operator. */
-  CXBinaryOperator_XorAssign,
+  CXBinaryOperator_XorAssign = 31,
   /** Bitwise OR assignment operator. */
-  CXBinaryOperator_OrAssign,
+  CXBinaryOperator_OrAssign = 32,
   /** Comma operator. */
-  CXBinaryOperator_Comma
+  CXBinaryOperator_Comma = 33,
+  CXBinaryOperator_Last = CXBinaryOperator_Comma
 };
 
 /**
diff --git a/clang/tools/c-index-test/c-index-test.c b/clang/tools/c-index-test/c-index-test.c
index 7711df3fd9209..35255a9d7f871 100644
--- a/clang/tools/c-index-test/c-index-test.c
+++ b/clang/tools/c-index-test/c-index-test.c
@@ -1863,14 +1863,14 @@ static enum CXChildVisitResult PrintTypeSize(CXCursor cursor, CXCursor p,
 static enum CXChildVisitResult PrintBinOps(CXCursor C, CXCursor p,
                                            CXClientData d) {
   enum CXCursorKind ck = clang_getCursorKind(C);
-  enum CX_BinaryOperatorKind bok;
+  enum CXBinaryOperatorKind bok;
   CXString opstr;
   if (ck != CXCursor_BinaryOperator && ck != CXCursor_CompoundAssignOperator)
     return CXChildVisit_Recurse;
 
   PrintCursor(C, NULL);
-  bok = clang_Cursor_getBinaryOpcode(C);
-  opstr = clang_Cursor_getBinaryOpcodeStr(bok);
+  bok = clang_getCursorBinaryOperatorKind(C);
+  opstr = clang_getBinaryOperatorKindSpelling(bok);
   printf(" BinOp=%s %d\n", clang_getCString(opstr), bok);
   clang_disposeString(opstr);
   return CXChildVisit_Recurse;
diff --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp
index e498a875bbbe8..e091d89c32265 100644
--- a/clang/tools/libclang/CIndex.cpp
+++ b/clang/tools/libclang/CIndex.cpp
@@ -5442,7 +5442,8 @@ CXString clang_getCursorSpelling(CXCursor C) {
 
     if (C.kind == CXCursor_BinaryOperator ||
         C.kind == CXCursor_CompoundAssignOperator) {
-      return clang_Cursor_getBinaryOpcodeStr(clang_Cursor_getBinaryOpcode(C));
+      return clang_getBinaryOperatorKindSpelling
+        (clang_getCursorBinaryOperatorKind(C));
     }
 
     const Decl *D = getDeclFromExpr(getCursorExpr(C));
@@ -9210,35 +9211,6 @@ unsigned clang_Cursor_isExternalSymbol(CXCursor C, CXString *language,
   return 0;
 }
 
-enum CX_BinaryOperatorKind clang_Cursor_getBinaryOpcode(CXCursor C) {
-  if (C.kind != CXCursor_BinaryOperator &&
-      C.kind != CXCursor_CompoundAssignOperator) {
-    return CX_BO_Invalid;
-  }
-
-  const Expr *D = getCursorExpr(C);
-  if (const auto *BinOp = dyn_cast<BinaryOperator>(D)) {
-    switch (BinOp->getOpcode()) {
-#define BINARY_OPERATION(Name, Spelling)                                       \
-  case BO_##Name:                                                              \
-    return CX_BO_##Name;
-#include "clang/AST/OperationKinds.def"
-    }
-  }
-
-  return CX_BO_Invalid;
-}
-
-CXString clang_Cursor_getBinaryOpcodeStr(enum CX_BinaryOperatorKind Op) {
-  if (Op > CX_BO_LAST)
-    return cxstring::createEmpty();
-
-  return cxstring::createDup(
-      // BinaryOperator::getOpcodeStr has no case for CX_BO_Invalid,
-      // so subtract 1
-      BinaryOperator::getOpcodeStr(static_cast<BinaryOperatorKind>(Op - 1)));
-}
-
 CXSourceRange clang_Cursor_getCommentRange(CXCursor C) {
   if (!clang_isDeclaration(C.kind))
     return clang_getNullRange();
@@ -10111,7 +10083,10 @@ cxindex::Logger::~Logger() {
 }
 
 CXString clang_getBinaryOperatorKindSpelling(enum CXBinaryOperatorKind kind) {
-  return cxstring::createRef(
+   if (kind > CXBinaryOperator_Last)
+    return cxstring::createEmpty();
+
+   return cxstring::createDup(
       BinaryOperator::getOpcodeStr(static_cast<BinaryOperatorKind>(kind - 1)));
 }
 
diff --git a/clang/tools/libclang/libclang.map b/clang/tools/libclang/libclang.map
index 07471ca42c97e..491579846e540 100644
--- a/clang/tools/libclang/libclang.map
+++ b/clang/tools/libclang/libclang.map
@@ -423,12 +423,6 @@ LLVM_17 {
     clang_getCursorUnaryOperatorKind;
 };
 
-LLVM_19 {
-  global:
-    clang_Cursor_getBinaryOpcode;
-    clang_Cursor_getBinaryOpcodeStr;
-};
-
 LLVM_20 {
   global:
     clang_getOffsetOfBase;

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[DeinAlptraum]

Oh wow, what an oversight. Thank you for this PR!
Looks good to me for the most part, it just needs a release note (especially for the breaking change).
I'd also like @Endilll to sign off on this as well

[DeinAlptraum]

I don't think we should touch sections of this file from older versions, but I'm still not entirely sure what it does tbh. @Endilll what do you think

[Jugst3r]

I think we have to remove it as this defines the list of symbols exposed in the DLL. As those symbols no longer exist with this MR, they must be removed

[Endilll]

libclang's API is called stable for a reason.
It's too late to remove anything. The best we can do is to implement one function in terms of another to avoid duplication on implementation side, and put [[deprecated("use the other one")]] on the one we think shouldn't be used.

[AaronBallman]

libclang's API is called stable for a reason. It's too late to remove anything. The best we can do is to implement one function in terms of another to avoid duplication on implementation side, and put [[deprecated("use the other one")]] on the one we think shouldn't be used.

+1 (and sorry for approving the changes that led us to this place: #98489).

[Jugst3r]

Sure, I did what @Endilll suggested. It is kind of unfortunate to have such similar type names :/. I don't think I can use the [[attribute]] C++-style syntax, maybe there is another syntax I am not aware of, but for the mean time I put the deprecated inside the comments.

[DeinAlptraum]

@Endilll if this fulfills your requested changes, can this be merged?

[AaronBallman]

LGTM!

[Endilll]

I talked to Aaron offline whether there's a better way to mark duplicates as deprecated, but given the variety of C compilers that can consume this header, we don't think there's a better way than a doxygen comment

[Endilll]

@Jugst3r Let us know if this is ready to be merged.