[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
arphaman added a comment. Recommitted in r305221. https://reviews.llvm.org/D33478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
arphaman added a comment. Thanks, I'll recommit today. https://reviews.llvm.org/D33478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
rdwampler updated this revision to Diff 102150.
rdwampler added a comment.
I was able to build and test this on a linux box. The issue was the whitespace
surrounding the regex.
On Linux, `(unavailable)` is not present. I.e,
`FunctionDecl=foo:3:6 (ios, introduced=3.2, deprecated=4.1) (macos,
introduced=10.4, deprecated=10.5, obsoleted=10.7)`
vs on macOS
`FunctionDecl=foo:3:6 (unavailable) (ios, introduced=3.2, deprecated=4.1)
(unavailable) (macos, introduced=10.4, deprecated=10.5, obsoleted=10.7)`
I changed it to just match any characters between the function declaration and
the availabilities.
Sorry for the noise.
https://reviews.llvm.org/D33478
Files:
test/Index/availability.c
tools/libclang/CIndex.cpp
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -7216,15 +7216,11 @@
return Out;
}
-static int getCursorPlatformAvailabilityForDecl(const Decl *D,
-int *always_deprecated,
-CXString *deprecated_message,
-int *always_unavailable,
-CXString *unavailable_message,
- CXPlatformAvailability *availability,
-int availability_size) {
+static void getCursorPlatformAvailabilityForDecl(
+const Decl *D, int *always_deprecated, CXString *deprecated_message,
+int *always_unavailable, CXString *unavailable_message,
+SmallVectorImpl &AvailabilityAttrs) {
bool HadAvailAttr = false;
- int N = 0;
for (auto A : D->attrs()) {
if (DeprecatedAttr *Deprecated = dyn_cast(A)) {
HadAvailAttr = true;
@@ -7236,7 +7232,7 @@
}
continue;
}
-
+
if (UnavailableAttr *Unavailable = dyn_cast(A)) {
HadAvailAttr = true;
if (always_unavailable)
@@ -7247,38 +7243,71 @@
}
continue;
}
-
+
if (AvailabilityAttr *Avail = dyn_cast(A)) {
+ AvailabilityAttrs.push_back(Avail);
HadAvailAttr = true;
- if (N < availability_size) {
-availability[N].Platform
- = cxstring::createDup(Avail->getPlatform()->getName());
-availability[N].Introduced = convertVersion(Avail->getIntroduced());
-availability[N].Deprecated = convertVersion(Avail->getDeprecated());
-availability[N].Obsoleted = convertVersion(Avail->getObsoleted());
-availability[N].Unavailable = Avail->getUnavailable();
-availability[N].Message = cxstring::createDup(Avail->getMessage());
- }
- ++N;
}
}
if (!HadAvailAttr)
if (const EnumConstantDecl *EnumConst = dyn_cast(D))
return getCursorPlatformAvailabilityForDecl(
-cast(EnumConst->getDeclContext()),
- always_deprecated,
- deprecated_message,
- always_unavailable,
- unavailable_message,
- availability,
- availability_size);
-
- return N;
+ cast(EnumConst->getDeclContext()), always_deprecated,
+ deprecated_message, always_unavailable, unavailable_message,
+ AvailabilityAttrs);
+
+ if (AvailabilityAttrs.empty())
+return;
+
+ std::sort(AvailabilityAttrs.begin(), AvailabilityAttrs.end(),
+[](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+ return LHS->getPlatform() > RHS->getPlatform();
+});
+ ASTContext &Ctx = D->getASTContext();
+ auto It = std::unique(
+ AvailabilityAttrs.begin(), AvailabilityAttrs.end(),
+ [&Ctx](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+if (LHS->getPlatform() != RHS->getPlatform())
+ return false;
+
+if (LHS->getIntroduced() == RHS->getIntroduced() &&
+LHS->getDeprecated() == RHS->getDeprecated() &&
+LHS->getObsoleted() == RHS->getObsoleted() &&
+LHS->getMessage() == RHS->getMessage() &&
+LHS->getReplacement() == RHS->getReplacement())
+ return true;
+
+if ((!LHS->getIntroduced().empty() && !RHS->getIntroduced().empty()) ||
+(!LHS->getDeprecated().empty() && !RHS->getDeprecated().empty()) ||
+(!LHS->getObsoleted().empty() && !RHS->getObsoleted().empty()))
+ return false;
+
+if (LHS->getIntroduced().empty() && !RHS->getIntroduced().empty())
+ LHS->setIntroduced(Ctx, RHS->getIntroduced());
+
+if (LHS->getDeprecated().empty() && !RHS->getDeprecated().empty()) {
+ LHS->setDeprecated(Ctx, RHS->getDeprecated());
+ if (LHS->getMessage().empty
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
rdwampler added a comment. Alex, I will look into it. Thanks! Repository: rL LLVM https://reviews.llvm.org/D33478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
arphaman added a comment. Ronald, I had to revert the commit as the `availability.c` test was failing on Linux (e.g. http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/12532). Can you please look into that? I think you might have to got back to the `CHECK-1` and `CHECK-2` thing Repository: rL LLVM https://reviews.llvm.org/D33478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
This revision was automatically updated to reflect the committed changes.
Closed by commit rL305117: [libclang] Merge multiple availability clauses when
getting the platform's (authored by arphaman).
Changed prior to commit:
https://reviews.llvm.org/D33478?vs=102050&id=102074#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33478
Files:
cfe/trunk/test/Index/availability.c
cfe/trunk/tools/libclang/CIndex.cpp
Index: cfe/trunk/test/Index/availability.c
===
--- cfe/trunk/test/Index/availability.c
+++ cfe/trunk/test/Index/availability.c
@@ -8,13 +8,15 @@
enum {
old_enum_plat
-} __attribute__((availability(macosx,introduced=10.4,deprecated=10.5,obsoleted=10.7)
+} __attribute__((availability(macosx,introduced=10.4,deprecated=10.5,obsoleted=10.7)));
-// RUN: c-index-test -test-load-source all %s > %t
-// RUN: FileCheck -check-prefix=CHECK-1 %s < %t
-// RUN: FileCheck -check-prefix=CHECK-2 %s < %t
-// CHECK-1: (ios, introduced=3.2, deprecated=4.1)
-// CHECK-2: (macos, introduced=10.4, deprecated=10.5, obsoleted=10.7)
+void bar(void) __attribute__((availability(macosx,introduced=10.4))) __attribute__((availability(macosx,obsoleted=10.6))) __attribute__((availability(ios,introduced=3.2))) __attribute__((availability(macosx,deprecated=10.5,message="use foobar")));
-// CHECK-2: EnumConstantDecl=old_enum:6:3 (Definition) (deprecated)
-// CHECK-2: EnumConstantDecl=old_enum_plat:10:3 {{.*}} (macos, introduced=10.4, deprecated=10.5, obsoleted=10.7)
+void bar2(void) __attribute__((availability(macosx,introduced=10.4,deprecated=10.5,obsoleted=10.7))) __attribute__((availability(ios,introduced=3.2,deprecated=10.0))) __attribute__((availability(macosx,introduced=10.4,deprecated=10.5,obsoleted=10.7))) __attribute__((availability(ios,introduced=3.2,deprecated=10.0)));
+
+// RUN: c-index-test -test-load-source all %s | FileCheck %s
+// CHECK: FunctionDecl=foo:3:6 {{.*}} (ios, introduced=3.2, deprecated=4.1) (macos, introduced=10.4, deprecated=10.5, obsoleted=10.7)
+// CHECK: EnumConstantDecl=old_enum:6:3 (Definition) (deprecated)
+// CHECK: EnumConstantDecl=old_enum_plat:10:3 {{.*}} (macos, introduced=10.4, deprecated=10.5, obsoleted=10.7)
+// CHECK: FunctionDecl=bar:13:6 {{.*}} (ios, introduced=3.2) (macos, introduced=10.4, deprecated=10.5, obsoleted=10.6, message="use foobar")
+// CHECK: FunctionDecl=bar2:15:6 {{.*}} (ios, introduced=3.2, deprecated=10.0) (macos, introduced=10.4, deprecated=10.5, obsoleted=10.7)
Index: cfe/trunk/tools/libclang/CIndex.cpp
===
--- cfe/trunk/tools/libclang/CIndex.cpp
+++ cfe/trunk/tools/libclang/CIndex.cpp
@@ -7216,15 +7216,11 @@
return Out;
}
-static int getCursorPlatformAvailabilityForDecl(const Decl *D,
-int *always_deprecated,
-CXString *deprecated_message,
-int *always_unavailable,
-CXString *unavailable_message,
- CXPlatformAvailability *availability,
-int availability_size) {
+static void getCursorPlatformAvailabilityForDecl(
+const Decl *D, int *always_deprecated, CXString *deprecated_message,
+int *always_unavailable, CXString *unavailable_message,
+SmallVectorImpl &AvailabilityAttrs) {
bool HadAvailAttr = false;
- int N = 0;
for (auto A : D->attrs()) {
if (DeprecatedAttr *Deprecated = dyn_cast(A)) {
HadAvailAttr = true;
@@ -7236,7 +7232,7 @@
}
continue;
}
-
+
if (UnavailableAttr *Unavailable = dyn_cast(A)) {
HadAvailAttr = true;
if (always_unavailable)
@@ -7247,38 +7243,71 @@
}
continue;
}
-
+
if (AvailabilityAttr *Avail = dyn_cast(A)) {
+ AvailabilityAttrs.push_back(Avail);
HadAvailAttr = true;
- if (N < availability_size) {
-availability[N].Platform
- = cxstring::createDup(Avail->getPlatform()->getName());
-availability[N].Introduced = convertVersion(Avail->getIntroduced());
-availability[N].Deprecated = convertVersion(Avail->getDeprecated());
-availability[N].Obsoleted = convertVersion(Avail->getObsoleted());
-availability[N].Unavailable = Avail->getUnavailable();
-availability[N].Message = cxstring::createDup(Avail->getMessage());
- }
- ++N;
}
}
if (!HadAvailAttr)
if (const EnumConstantDecl *EnumConst = dyn_cast(D))
return getCursorPlatformAvailabilityForDecl(
-cast(EnumConst->getDeclContext()),
- always_deprecated,
- deprecated_message,
- always_unavailable,
-
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
rdwampler marked 2 inline comments as done. rdwampler added a comment. I don't have commit access, but I would grateful if you can commit it. https://reviews.llvm.org/D33478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
arphaman added a comment. Do you have commit access or shall I commit it on your behalf? https://reviews.llvm.org/D33478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D33478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
rdwampler updated this revision to Diff 102050.
rdwampler added a comment.
Remove an unnecessary check when determining a mismatch of availabilities
Simplify invocation of FileCheck in availability.c
https://reviews.llvm.org/D33478
Files:
test/Index/availability.c
tools/libclang/CIndex.cpp
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -7200,15 +7200,11 @@
return Out;
}
-static int getCursorPlatformAvailabilityForDecl(const Decl *D,
-int *always_deprecated,
-CXString *deprecated_message,
-int *always_unavailable,
-CXString *unavailable_message,
- CXPlatformAvailability *availability,
-int availability_size) {
+static void getCursorPlatformAvailabilityForDecl(
+const Decl *D, int *always_deprecated, CXString *deprecated_message,
+int *always_unavailable, CXString *unavailable_message,
+SmallVectorImpl &AvailabilityAttrs) {
bool HadAvailAttr = false;
- int N = 0;
for (auto A : D->attrs()) {
if (DeprecatedAttr *Deprecated = dyn_cast(A)) {
HadAvailAttr = true;
@@ -7220,7 +7216,7 @@
}
continue;
}
-
+
if (UnavailableAttr *Unavailable = dyn_cast(A)) {
HadAvailAttr = true;
if (always_unavailable)
@@ -7231,38 +7227,71 @@
}
continue;
}
-
+
if (AvailabilityAttr *Avail = dyn_cast(A)) {
+ AvailabilityAttrs.push_back(Avail);
HadAvailAttr = true;
- if (N < availability_size) {
-availability[N].Platform
- = cxstring::createDup(Avail->getPlatform()->getName());
-availability[N].Introduced = convertVersion(Avail->getIntroduced());
-availability[N].Deprecated = convertVersion(Avail->getDeprecated());
-availability[N].Obsoleted = convertVersion(Avail->getObsoleted());
-availability[N].Unavailable = Avail->getUnavailable();
-availability[N].Message = cxstring::createDup(Avail->getMessage());
- }
- ++N;
}
}
if (!HadAvailAttr)
if (const EnumConstantDecl *EnumConst = dyn_cast(D))
return getCursorPlatformAvailabilityForDecl(
-cast(EnumConst->getDeclContext()),
- always_deprecated,
- deprecated_message,
- always_unavailable,
- unavailable_message,
- availability,
- availability_size);
-
- return N;
+ cast(EnumConst->getDeclContext()), always_deprecated,
+ deprecated_message, always_unavailable, unavailable_message,
+ AvailabilityAttrs);
+
+ if (AvailabilityAttrs.empty())
+return;
+
+ std::sort(AvailabilityAttrs.begin(), AvailabilityAttrs.end(),
+[](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+ return LHS->getPlatform() > RHS->getPlatform();
+});
+ ASTContext &Ctx = D->getASTContext();
+ auto It = std::unique(
+ AvailabilityAttrs.begin(), AvailabilityAttrs.end(),
+ [&Ctx](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+if (LHS->getPlatform() != RHS->getPlatform())
+ return false;
+
+if (LHS->getIntroduced() == RHS->getIntroduced() &&
+LHS->getDeprecated() == RHS->getDeprecated() &&
+LHS->getObsoleted() == RHS->getObsoleted() &&
+LHS->getMessage() == RHS->getMessage() &&
+LHS->getReplacement() == RHS->getReplacement())
+ return true;
+
+if ((!LHS->getIntroduced().empty() && !RHS->getIntroduced().empty()) ||
+(!LHS->getDeprecated().empty() && !RHS->getDeprecated().empty()) ||
+(!LHS->getObsoleted().empty() && !RHS->getObsoleted().empty()))
+ return false;
+
+if (LHS->getIntroduced().empty() && !RHS->getIntroduced().empty())
+ LHS->setIntroduced(Ctx, RHS->getIntroduced());
+
+if (LHS->getDeprecated().empty() && !RHS->getDeprecated().empty()) {
+ LHS->setDeprecated(Ctx, RHS->getDeprecated());
+ if (LHS->getMessage().empty())
+LHS->setMessage(Ctx, RHS->getMessage());
+ if (LHS->getReplacement().empty())
+LHS->setReplacement(Ctx, RHS->getReplacement());
+}
+
+if (LHS->getObsoleted().empty() && !RHS->getObsoleted().empty()) {
+ LHS->setObsoleted(Ctx, RHS->getObsoleted());
+ if (LHS->getMessage().empty())
+LHS->setMessage(Ctx, RHS->getMessage());
+ if (LHS->getRep
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
arphaman added inline comments. Comment at: test/Index/availability.c:20 // CHECK-2: (macos, introduced=10.4, deprecated=10.5, obsoleted=10.7) // CHECK-2: EnumConstantDecl=old_enum:6:3 (Definition) (deprecated) rdwampler wrote: > Can we run `FileCheck` once now? I believe the `CHECK-1` and `CHECK-2` were > added since there were no particular order for the availabilities. With this > patch the order is guarantee to be stable. Sure, if the test passes we can use one `FileCheck` invocation. We can also use `CHECK-DAG:` to check for strings without obeying a specific order. https://reviews.llvm.org/D33478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
rdwampler marked an inline comment as done. rdwampler added inline comments. Comment at: test/Index/availability.c:20 // CHECK-2: (macos, introduced=10.4, deprecated=10.5, obsoleted=10.7) // CHECK-2: EnumConstantDecl=old_enum:6:3 (Definition) (deprecated) Can we run `FileCheck` once now? I believe the `CHECK-1` and `CHECK-2` were added since there were no particular order for the availabilities. With this patch the order is guarantee to be stable. Comment at: tools/libclang/CIndex.cpp:7268 +(!LHS->getObsoleted().empty() && !RHS->getObsoleted().empty()) || +(!LHS->getMessage().empty() && !RHS->getMessage().empty())) + return false; arphaman wrote: > I think that we don't really need the `(!LHS->getMessage().empty() && > !RHS->getMessage().empty())` check here since message has to be either in a > deprecated or obsoleted clause, so we should already handle that with > previous checks. Agreed. https://reviews.llvm.org/D33478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
arphaman added a comment. This looks better, it's almost ready. A couple of small requests: Comment at: tools/libclang/CIndex.cpp:7262 +LHS->getMessage() == RHS->getMessage() && +LHS->getReplacement() == RHS->getReplacement()) + return true; We should also have a test that verifies that we merge identical availabilities. Comment at: tools/libclang/CIndex.cpp:7268 +(!LHS->getObsoleted().empty() && !RHS->getObsoleted().empty()) || +(!LHS->getMessage().empty() && !RHS->getMessage().empty())) + return false; I think that we don't really need the `(!LHS->getMessage().empty() && !RHS->getMessage().empty())` check here since message has to be either in a deprecated or obsoleted clause, so we should already handle that with previous checks. https://reviews.llvm.org/D33478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
rdwampler updated this revision to Diff 101787.
rdwampler marked 6 inline comments as done.
rdwampler added a comment.
This should resolve the bug (a conditional was inverted) in the last revision
along with the other changes requested.
https://reviews.llvm.org/D33478
Files:
test/Index/availability.c
tools/libclang/CIndex.cpp
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -7200,15 +7200,11 @@
return Out;
}
-static int getCursorPlatformAvailabilityForDecl(const Decl *D,
-int *always_deprecated,
-CXString *deprecated_message,
-int *always_unavailable,
-CXString *unavailable_message,
- CXPlatformAvailability *availability,
-int availability_size) {
+static void getCursorPlatformAvailabilityForDecl(
+const Decl *D, int *always_deprecated, CXString *deprecated_message,
+int *always_unavailable, CXString *unavailable_message,
+SmallVectorImpl &AvailabilityAttrs) {
bool HadAvailAttr = false;
- int N = 0;
for (auto A : D->attrs()) {
if (DeprecatedAttr *Deprecated = dyn_cast(A)) {
HadAvailAttr = true;
@@ -7220,7 +7216,7 @@
}
continue;
}
-
+
if (UnavailableAttr *Unavailable = dyn_cast(A)) {
HadAvailAttr = true;
if (always_unavailable)
@@ -7231,38 +7227,72 @@
}
continue;
}
-
+
if (AvailabilityAttr *Avail = dyn_cast(A)) {
+ AvailabilityAttrs.push_back(Avail);
HadAvailAttr = true;
- if (N < availability_size) {
-availability[N].Platform
- = cxstring::createDup(Avail->getPlatform()->getName());
-availability[N].Introduced = convertVersion(Avail->getIntroduced());
-availability[N].Deprecated = convertVersion(Avail->getDeprecated());
-availability[N].Obsoleted = convertVersion(Avail->getObsoleted());
-availability[N].Unavailable = Avail->getUnavailable();
-availability[N].Message = cxstring::createDup(Avail->getMessage());
- }
- ++N;
}
}
if (!HadAvailAttr)
if (const EnumConstantDecl *EnumConst = dyn_cast(D))
return getCursorPlatformAvailabilityForDecl(
-cast(EnumConst->getDeclContext()),
- always_deprecated,
- deprecated_message,
- always_unavailable,
- unavailable_message,
- availability,
- availability_size);
-
- return N;
+ cast(EnumConst->getDeclContext()), always_deprecated,
+ deprecated_message, always_unavailable, unavailable_message,
+ AvailabilityAttrs);
+
+ if (AvailabilityAttrs.empty())
+return;
+
+ std::sort(AvailabilityAttrs.begin(), AvailabilityAttrs.end(),
+[](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+ return LHS->getPlatform() > RHS->getPlatform();
+});
+ ASTContext &Ctx = D->getASTContext();
+ auto It = std::unique(
+ AvailabilityAttrs.begin(), AvailabilityAttrs.end(),
+ [&Ctx](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+if (LHS->getPlatform() != RHS->getPlatform())
+ return false;
+
+if (LHS->getIntroduced() == RHS->getIntroduced() &&
+LHS->getDeprecated() == RHS->getDeprecated() &&
+LHS->getObsoleted() == RHS->getObsoleted() &&
+LHS->getMessage() == RHS->getMessage() &&
+LHS->getReplacement() == RHS->getReplacement())
+ return true;
+
+if ((!LHS->getIntroduced().empty() && !RHS->getIntroduced().empty()) ||
+(!LHS->getDeprecated().empty() && !RHS->getDeprecated().empty()) ||
+(!LHS->getObsoleted().empty() && !RHS->getObsoleted().empty()) ||
+(!LHS->getMessage().empty() && !RHS->getMessage().empty()))
+ return false;
+
+if (LHS->getIntroduced().empty() && !RHS->getIntroduced().empty())
+ LHS->setIntroduced(Ctx, RHS->getIntroduced());
+
+if (LHS->getDeprecated().empty() && !RHS->getDeprecated().empty()) {
+ LHS->setDeprecated(Ctx, RHS->getDeprecated());
+ if (LHS->getMessage().empty())
+LHS->setMessage(Ctx, RHS->getMessage());
+ if (LHS->getReplacement().empty())
+LHS->setReplacement(Ctx, RHS->getReplacement());
+}
+
+if (LHS->getObsoleted().empty() && !RHS->getObsoleted().empty()) {
+ LHS->setObsoleted(Ctx, RHS->getObsoleted());
+
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
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(D))
- return getCursorPlatformAvailabilityForDecl(
-
cast(EnumConst->getDeclContext()),
- always_deprecated,
- deprecated_message,
- always_unavailable,
- unavailable_message,
- availability,
- availability_size);
-
- return N;
+ getCursorPlatformAvailabilityForDecl(
+ cast(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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
rdwampler updated this revision to Diff 101569.
rdwampler added a comment.
Rearrange `if` statements in `getCursorPlatformAvailabilityForDecl` to return
early if we do not need to merge availability attributes.
Use ranged for loop.
https://reviews.llvm.org/D33478
Files:
test/Index/availability.c
tools/libclang/CIndex.cpp
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -7200,15 +7200,11 @@
return Out;
}
-static int getCursorPlatformAvailabilityForDecl(const Decl *D,
-int *always_deprecated,
-CXString *deprecated_message,
-int *always_unavailable,
-CXString *unavailable_message,
- CXPlatformAvailability *availability,
-int availability_size) {
+static void getCursorPlatformAvailabilityForDecl(
+const Decl *D, int *always_deprecated, CXString *deprecated_message,
+int *always_unavailable, CXString *unavailable_message,
+SmallVectorImpl &AvailabilityAttrs) {
bool HadAvailAttr = false;
- int N = 0;
for (auto A : D->attrs()) {
if (DeprecatedAttr *Deprecated = dyn_cast(A)) {
HadAvailAttr = true;
@@ -7220,7 +7216,7 @@
}
continue;
}
-
+
if (UnavailableAttr *Unavailable = dyn_cast(A)) {
HadAvailAttr = true;
if (always_unavailable)
@@ -7231,38 +7227,72 @@
}
continue;
}
-
+
if (AvailabilityAttr *Avail = dyn_cast(A)) {
+ AvailabilityAttrs.push_back(Avail);
HadAvailAttr = true;
- if (N < availability_size) {
-availability[N].Platform
- = cxstring::createDup(Avail->getPlatform()->getName());
-availability[N].Introduced = convertVersion(Avail->getIntroduced());
-availability[N].Deprecated = convertVersion(Avail->getDeprecated());
-availability[N].Obsoleted = convertVersion(Avail->getObsoleted());
-availability[N].Unavailable = Avail->getUnavailable();
-availability[N].Message = cxstring::createDup(Avail->getMessage());
- }
- ++N;
}
}
if (!HadAvailAttr)
if (const EnumConstantDecl *EnumConst = dyn_cast(D))
- return getCursorPlatformAvailabilityForDecl(
-cast(EnumConst->getDeclContext()),
- always_deprecated,
- deprecated_message,
- always_unavailable,
- unavailable_message,
- availability,
- availability_size);
-
- return N;
+ getCursorPlatformAvailabilityForDecl(
+ cast(EnumConst->getDeclContext()), always_deprecated,
+ deprecated_message, always_unavailable, unavailable_message,
+ AvailabilityAttrs);
+
+ if (AvailabilityAttrs.empty())
+return;
+
+ std::sort(AvailabilityAttrs.begin(), AvailabilityAttrs.end(),
+[](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+ return LHS->getPlatform() > RHS->getPlatform();
+});
+ ASTContext &Ctx = D->getASTContext();
+ auto It = std::unique(
+ AvailabilityAttrs.begin(), AvailabilityAttrs.end(),
+ [&Ctx](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+if (LHS->getPlatform() == RHS->getPlatform()) {
+ if (LHS->getIntroduced() == RHS->getIntroduced() &&
+ LHS->getDeprecated() == RHS->getDeprecated() &&
+ LHS->getObsoleted() == RHS->getObsoleted() &&
+ LHS->getMessage() == RHS->getMessage() &&
+ LHS->getReplacement() == RHS->getReplacement())
+return true;
+
+ if ((!LHS->getIntroduced().empty() &&
+ !RHS->getIntroduced().empty()) ||
+ (!LHS->getDeprecated().empty() &&
+ !RHS->getDeprecated().empty()) ||
+ (!LHS->getObsoleted().empty() && !RHS->getObsoleted().empty()) ||
+ (!LHS->getMessage().empty() && !RHS->getMessage().empty()))
+return false;
+
+ if (LHS->getIntroduced().empty() && !RHS->getIntroduced().empty())
+LHS->setIntroduced(Ctx, RHS->getIntroduced());
+
+ if (LHS->getDeprecated().empty() && !RHS->getDeprecated().empty()) {
+LHS->setDeprecated(Ctx, RHS->getDeprecated());
+if (!LHS->getMessage().empty())
+ LHS->setMessage(Ctx, RHS->getMessage());
+if (!LHS->getReplacement().empty())
+ LHS->setReplacement(Ctx, RHS->getReplacement());
+ }
+
+ if (LHS->getObsoleted().empty() && !RHS->
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
arphaman added inline comments.
Comment at: tools/libclang/CIndex.cpp:7322
+
+ for (int I = 0, E = AvailabilityAttrs.size(); I < E && I < availability_size;
+ ++I) {
rdwampler wrote:
> arphaman wrote:
> > You can use a ranged for loop here if you use `take_front`, e.g.
> >
> > ```
> > for (const auto *Avail : AvailabilityAttrs.take_front(availability_size))
> > ```
> I would need to covert this to an `ArrayRef`, I believe. Also, I would need
> to check `availability_size` is in bounds. Would something like the following
> be preferred:
> ```
> int N = 0;
> for (const auto *Avail : AvailabilityAttrs) {
> if (N < availability_size) {
> // populate availability
> N++;
> }
> }
> ```
Right, sorry I forgot about the `N`.
I think that you can use
`llvm::enumerate(llvm::makeArrayRef(AvailabilityAttrs).take_front(availability_size))`
to get rid of `N` as well, as you can the index and the attribute pointer from
the loop variable.
Btw, The `take_front(N)` will ensure that you will only iterate either over the
first N attributes or all of the attributes if `N > AvailabilityAttrs.size()`,
so you won't need to check `N` in the loop.
https://reviews.llvm.org/D33478
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
rdwampler added inline comments.
Comment at: tools/libclang/CIndex.cpp:7322
+
+ for (int I = 0, E = AvailabilityAttrs.size(); I < E && I < availability_size;
+ ++I) {
arphaman wrote:
> You can use a ranged for loop here if you use `take_front`, e.g.
>
> ```
> for (const auto *Avail : AvailabilityAttrs.take_front(availability_size))
> ```
I would need to covert this to an `ArrayRef`, I believe. Also, I would need to
check `availability_size` is in bounds. Would something like the following be
preferred:
```
int N = 0;
for (const auto *Avail : AvailabilityAttrs) {
if (N < availability_size) {
// populate availability
N++;
}
}
```
https://reviews.llvm.org/D33478
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
arphaman added inline comments.
Comment at: tools/libclang/CIndex.cpp:7287
+
if (!HadAvailAttr)
if (const EnumConstantDecl *EnumConst = dyn_cast(D))
I think that you can move this `if` before the new `if`, and convert the new
`if` to be an `if (AvailabilityAttrs.empty())` that returns early. Then you can
do the merging outside of the `if` at the end of the function.
Comment at: tools/libclang/CIndex.cpp:7322
+
+ for (int I = 0, E = AvailabilityAttrs.size(); I < E && I < availability_size;
+ ++I) {
You can use a ranged for loop here if you use `take_front`, e.g.
```
for (const auto *Avail : AvailabilityAttrs.take_front(availability_size))
```
https://reviews.llvm.org/D33478
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
rdwampler created this revision.
https://reviews.llvm.org/D33478
Files:
test/Index/availability.c
tools/libclang/CIndex.cpp
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -7200,15 +7200,12 @@
return Out;
}
-static int getCursorPlatformAvailabilityForDecl(const Decl *D,
-int *always_deprecated,
-CXString *deprecated_message,
-int *always_unavailable,
-CXString *unavailable_message,
- CXPlatformAvailability *availability,
-int availability_size) {
+static void getCursorPlatformAvailabilityForDecl(
+const Decl *D, int *always_deprecated, CXString *deprecated_message,
+int *always_unavailable, CXString *unavailable_message,
+SmallVectorImpl &AvailabilityAttrs) {
bool HadAvailAttr = false;
- int N = 0;
+ ASTContext &Ctx = D->getASTContext();
for (auto A : D->attrs()) {
if (DeprecatedAttr *Deprecated = dyn_cast(A)) {
HadAvailAttr = true;
@@ -7220,7 +7217,7 @@
}
continue;
}
-
+
if (UnavailableAttr *Unavailable = dyn_cast(A)) {
HadAvailAttr = true;
if (always_unavailable)
@@ -7231,38 +7228,71 @@
}
continue;
}
-
+
if (AvailabilityAttr *Avail = dyn_cast(A)) {
+ AvailabilityAttrs.push_back(Avail);
HadAvailAttr = true;
- if (N < availability_size) {
-availability[N].Platform
- = cxstring::createDup(Avail->getPlatform()->getName());
-availability[N].Introduced = convertVersion(Avail->getIntroduced());
-availability[N].Deprecated = convertVersion(Avail->getDeprecated());
-availability[N].Obsoleted = convertVersion(Avail->getObsoleted());
-availability[N].Unavailable = Avail->getUnavailable();
-availability[N].Message = cxstring::createDup(Avail->getMessage());
- }
- ++N;
}
}
+ if (!AvailabilityAttrs.empty()) {
+std::sort(AvailabilityAttrs.begin(), AvailabilityAttrs.end(),
+ [](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+return LHS->getPlatform() > RHS->getPlatform();
+ });
+auto It = std::unique(
+AvailabilityAttrs.begin(), AvailabilityAttrs.end(),
+[&Ctx](AvailabilityAttr *LHS, AvailabilityAttr *RHS) {
+ if (LHS->getPlatform() == RHS->getPlatform()) {
+if (LHS->getIntroduced() == RHS->getIntroduced() &&
+LHS->getDeprecated() == RHS->getDeprecated() &&
+LHS->getObsoleted() == RHS->getObsoleted() &&
+LHS->getMessage() == RHS->getMessage() &&
+LHS->getReplacement() == RHS->getReplacement())
+ return true;
+
+if ((!LHS->getIntroduced().empty() &&
+ !RHS->getIntroduced().empty()) ||
+(!LHS->getDeprecated().empty() &&
+ !RHS->getDeprecated().empty()) ||
+(!LHS->getObsoleted().empty() &&
+ !RHS->getObsoleted().empty()) ||
+(!LHS->getMessage().empty() && !RHS->getMessage().empty()))
+ return false;
+
+if (LHS->getIntroduced().empty() && !RHS->getIntroduced().empty())
+ LHS->setIntroduced(Ctx, RHS->getIntroduced());
+
+if (LHS->getDeprecated().empty() && !RHS->getDeprecated().empty()) {
+ LHS->setDeprecated(Ctx, RHS->getDeprecated());
+ if (!LHS->getMessage().empty())
+LHS->setMessage(Ctx, RHS->getMessage());
+ if (!LHS->getReplacement().empty())
+LHS->setReplacement(Ctx, RHS->getReplacement());
+}
+
+if (LHS->getObsoleted().empty() && !RHS->getObsoleted().empty()) {
+ LHS->setObsoleted(Ctx, RHS->getObsoleted());
+ if (!LHS->getMessage().empty())
+LHS->setMessage(Ctx, RHS->getMessage());
+}
+
+return true;
+ }
+ return false;
+});
+AvailabilityAttrs.erase(It, AvailabilityAttrs.end());
+ }
+
if (!HadAvailAttr)
if (const EnumConstantDecl *EnumConst = dyn_cast(D))
- return getCursorPlatformAvailabilityForDecl(
-cast(EnumConst->getDeclContext()),
- always_deprecated,
- deprecated_message,
- always_unavailable,
- unavailable_message,
- availability,
- availability_size);
-
- return N
