[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

It seems that there's a slight bug in the patch:

If I print the output of the following code using `c-index-test -test-load-source all`:

  void bar2(void)
  __attribute__((availability(macosx, introduced=10.4)))
  __attribute__((availability(macosx, deprecated=10.5, message="use x")));

I see availability that includes a deprecation message: `FunctionDecl=bar2:15:6 (deprecated)  (macos, introduced=10.4, deprecated=10.5, message="use x")`.
However, if I look at the output of the following code:

  void bar2(void)
  __attribute__((availability(macosx, deprecated=10.5, message="use x")))
  __attribute__((availability(macosx, introduced=10.4)));

I get something that doesn't include the message: `FunctionDecl=bar2:15:6 (deprecated)  (macos, introduced=10.4, deprecated=10.5)`.



================
Comment at: test/Index/availability.c:13
+
+void bar(void) __attribute__((availability(macosx,introduced=10.4))) __attribute__((availability(ios,introduced=3.2))) __attribute__((availability(macosx,deprecated=10.5,message="use foobar")));
 
----------------
Please add a test for merge of the `obsoleted` clause as well.


================
Comment at: tools/libclang/CIndex.cpp:7239
     if (const EnumConstantDecl *EnumConst = dyn_cast<EnumConstantDecl>(D))
-      return getCursorPlatformAvailabilityForDecl(
-                                        cast<Decl>(EnumConst->getDeclContext()),
-                                                  always_deprecated,
-                                                  deprecated_message,
-                                                  always_unavailable,
-                                                  unavailable_message,
-                                                  availability,
-                                                  availability_size);
-  
-  return N;
+      getCursorPlatformAvailabilityForDecl(
+          cast<Decl>(EnumConst->getDeclContext()), always_deprecated,
----------------
You should be able to keep the `return` here, since you will `sort` and `unique` the attributes in this call to `getCursorPlatformAvailabilityForDecl`.


================
Comment at: tools/libclang/CIndex.cpp:7255
+      [&Ctx](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+        if (LHS->getPlatform() == RHS->getPlatform()) {
+          if (LHS->getIntroduced() == RHS->getIntroduced() &&
----------------
Please invert this `if` and `return false` early instead of leaving it at the end of the lambda.


https://reviews.llvm.org/D33478