[clang] [clang-format] Remove duplicates in @property using std::set (PR #74235)
https://github.com/owenca closed https://github.com/llvm/llvm-project/pull/74235 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Remove duplicates in @property using std::set (PR #74235)
https://github.com/mydeveloperday approved this pull request. https://github.com/llvm/llvm-project/pull/74235 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Remove duplicates in @property using std::set (PR #74235)
https://github.com/owenca updated
https://github.com/llvm/llvm-project/pull/74235
>From 7323d9261fe8c876fe3a656a98e186af3dd0b2a0 Mon Sep 17 00:00:00 2001
From: Owen Pan
Date: Sun, 3 Dec 2023 01:41:57 -0800
Subject: [PATCH 1/2] [clang-format] Remove duplicates in @property using
std::set
Re-implement ObjCPropertyAttributeOrder using std::set for sorting and
removing duplicates. (We can't use llvm::SmallSet because it's unordered.)
---
.../ObjCPropertyAttributeOrderFixer.cpp | 116 ++
.../Format/ObjCPropertyAttributeOrderFixer.h | 4 +-
.../ObjCPropertyAttributeOrderFixerTest.cpp | 12 +-
3 files changed, 75 insertions(+), 57 deletions(-)
diff --git a/clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp
b/clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp
index 20108306f1039..57b19a62154d1 100644
--- a/clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp
+++ b/clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp
@@ -15,8 +15,6 @@
#include "ObjCPropertyAttributeOrderFixer.h"
-#include "llvm/ADT/Sequence.h"
-
#include
namespace clang {
@@ -25,11 +23,10 @@ namespace format {
ObjCPropertyAttributeOrderFixer::ObjCPropertyAttributeOrderFixer(
const Environment , const FormatStyle )
: TokenAnalyzer(Env, Style) {
-
// Create an "order priority" map to use to sort properties.
- unsigned index = 0;
+ unsigned Index = 0;
for (const auto : Style.ObjCPropertyAttributeOrder)
-SortOrderMap[Property] = index++;
+SortOrderMap[Property] = Index++;
}
struct ObjCPropertyEntry {
@@ -37,14 +34,9 @@ struct ObjCPropertyEntry {
StringRef Value; // eg, the "foo" of the attribute "getter=foo"
};
-static bool isObjCPropertyAttribute(const FormatToken *Tok) {
- // Most attributes look like identifiers, but `class` is a keyword.
- return Tok->isOneOf(tok::identifier, tok::kw_class);
-}
-
void ObjCPropertyAttributeOrderFixer::sortPropertyAttributes(
const SourceManager , tooling::Replacements ,
-const FormatToken *BeginTok, const FormatToken *EndTok) const {
+const FormatToken *BeginTok, const FormatToken *EndTok) {
assert(BeginTok);
assert(EndTok);
assert(EndTok->Previous);
@@ -53,8 +45,16 @@ void ObjCPropertyAttributeOrderFixer::sortPropertyAttributes(
if (BeginTok == EndTok || BeginTok->Next == EndTok)
return;
+ // Use a set to sort attributes and remove duplicates.
+ std::set Ordinals;
+
+ // Create a "remapping index" on how to reorder the attributes.
+ SmallVector Indices;
+
// Collect the attributes.
- SmallVector PropertyAttributes;
+ SmallVector PropertyAttributes;
+ bool HasDuplicates = false;
+ int Index = 0;
for (auto Tok = BeginTok; Tok != EndTok; Tok = Tok->Next) {
assert(Tok);
if (Tok->is(tok::comma)) {
@@ -62,13 +62,14 @@ void
ObjCPropertyAttributeOrderFixer::sortPropertyAttributes(
continue;
}
-if (!isObjCPropertyAttribute(Tok)) {
+// Most attributes look like identifiers, but `class` is a keyword.
+if (!Tok->isOneOf(tok::identifier, tok::kw_class)) {
// If we hit any other kind of token, just bail.
return;
}
-// Memoize the attribute. (Note that 'class' is a legal attribute!)
-PropertyAttributes.push_back({Tok->TokenText, StringRef{}});
+const StringRef Attribute{Tok->TokenText};
+StringRef Value;
// Also handle `getter=getFoo` attributes.
// (Note: no check needed against `EndTok`, since its type is not
@@ -82,49 +83,66 @@ void
ObjCPropertyAttributeOrderFixer::sortPropertyAttributes(
return;
}
Tok = Tok->Next;
- PropertyAttributes.back().Value = Tok->TokenText;
+ Value = Tok->TokenText;
+}
+
+auto It = SortOrderMap.find(Attribute);
+if (It == SortOrderMap.end())
+ It = SortOrderMap.insert({Attribute, SortOrderMap.size()}).first;
+
+// Sort the indices based on the priority stored in 'SortOrderMap'.
+const auto Ordinal = It->second;
+if (!Ordinals.insert(Ordinal).second) {
+ HasDuplicates = true;
+ continue;
}
+
+if (Ordinal >= Indices.size())
+ Indices.resize(Ordinal + 1);
+Indices[Ordinal] = Index++;
+
+// Memoize the attribute.
+PropertyAttributes.push_back({Attribute, Value});
}
- // There's nothing to do unless there's more than one attribute.
- if (PropertyAttributes.size() < 2)
-return;
+ if (!HasDuplicates) {
+// There's nothing to do unless there's more than one attribute.
+if (PropertyAttributes.size() < 2)
+ return;
- // Create a "remapping index" on how to reorder the attributes.
- SmallVector Indices =
- llvm::to_vector<8>(llvm::seq(0, PropertyAttributes.size()));
-
- // Sort the indices based on the priority stored in 'SortOrderMap'; use Max
- // for missing values.
- const auto SortOrderMax = Style.ObjCPropertyAttributeOrder.size();
- auto SortIndex = [&](const StringRef ) -> unsigned {
-auto I = SortOrderMap.find(Needle);
-return (I ==
[clang] [clang-format] Remove duplicates in @property using std::set (PR #74235)
https://github.com/jaredgrubb edited https://github.com/llvm/llvm-project/pull/74235 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Remove duplicates in @property using std::set (PR #74235)
@@ -171,18 +171,18 @@ TEST_F(ObjCPropertyAttributeOrderFixerTest,
HandlesDuplicatedAttributes) {
Style.ObjCPropertyAttributeOrder = {"a", "b", "c"};
// Just a dup and nothing else.
- verifyFormat("@property(a, a) int p;", Style);
+ verifyFormat("@property(a) int p;", "@property(a, a) int p;", Style);
// A dup and something else.
- verifyFormat("@property(a, a, b) int p;", "@property(a, b, a) int p;",
Style);
+ verifyFormat("@property(a, b) int p;", "@property(a, b, a) int p;", Style);
// Duplicates using `=`: stable-sort irrespective of their value.
jaredgrubb wrote:
Nit: the comment about stable-sort isn't really applicable now.
https://github.com/llvm/llvm-project/pull/74235
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Remove duplicates in @property using std::set (PR #74235)
https://github.com/jaredgrubb approved this pull request. Looks good to me! https://github.com/llvm/llvm-project/pull/74235 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Remove duplicates in @property using std::set (PR #74235)
owenca wrote: @jaredgrubb I can't add you to Reviewers. Maybe you can add yourself? https://github.com/llvm/llvm-project/pull/74235 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Remove duplicates in @property using std::set (PR #74235)
llvmbot wrote:
@llvm/pr-subscribers-clang-format
Author: Owen Pan (owenca)
Changes
Re-implement ObjCPropertyAttributeOrder using std::set for sorting and removing
duplicates. (We can't use llvm::SmallSet because it's unordered.)
---
Full diff: https://github.com/llvm/llvm-project/pull/74235.diff
3 Files Affected:
- (modified) clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp (+67-49)
- (modified) clang/lib/Format/ObjCPropertyAttributeOrderFixer.h (+2-2)
- (modified) clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp
(+6-6)
``diff
diff --git a/clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp
b/clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp
index 20108306f1039..57b19a62154d1 100644
--- a/clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp
+++ b/clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp
@@ -15,8 +15,6 @@
#include "ObjCPropertyAttributeOrderFixer.h"
-#include "llvm/ADT/Sequence.h"
-
#include
namespace clang {
@@ -25,11 +23,10 @@ namespace format {
ObjCPropertyAttributeOrderFixer::ObjCPropertyAttributeOrderFixer(
const Environment , const FormatStyle )
: TokenAnalyzer(Env, Style) {
-
// Create an "order priority" map to use to sort properties.
- unsigned index = 0;
+ unsigned Index = 0;
for (const auto : Style.ObjCPropertyAttributeOrder)
-SortOrderMap[Property] = index++;
+SortOrderMap[Property] = Index++;
}
struct ObjCPropertyEntry {
@@ -37,14 +34,9 @@ struct ObjCPropertyEntry {
StringRef Value; // eg, the "foo" of the attribute "getter=foo"
};
-static bool isObjCPropertyAttribute(const FormatToken *Tok) {
- // Most attributes look like identifiers, but `class` is a keyword.
- return Tok->isOneOf(tok::identifier, tok::kw_class);
-}
-
void ObjCPropertyAttributeOrderFixer::sortPropertyAttributes(
const SourceManager , tooling::Replacements ,
-const FormatToken *BeginTok, const FormatToken *EndTok) const {
+const FormatToken *BeginTok, const FormatToken *EndTok) {
assert(BeginTok);
assert(EndTok);
assert(EndTok->Previous);
@@ -53,8 +45,16 @@ void ObjCPropertyAttributeOrderFixer::sortPropertyAttributes(
if (BeginTok == EndTok || BeginTok->Next == EndTok)
return;
+ // Use a set to sort attributes and remove duplicates.
+ std::set Ordinals;
+
+ // Create a "remapping index" on how to reorder the attributes.
+ SmallVector Indices;
+
// Collect the attributes.
- SmallVector PropertyAttributes;
+ SmallVector PropertyAttributes;
+ bool HasDuplicates = false;
+ int Index = 0;
for (auto Tok = BeginTok; Tok != EndTok; Tok = Tok->Next) {
assert(Tok);
if (Tok->is(tok::comma)) {
@@ -62,13 +62,14 @@ void
ObjCPropertyAttributeOrderFixer::sortPropertyAttributes(
continue;
}
-if (!isObjCPropertyAttribute(Tok)) {
+// Most attributes look like identifiers, but `class` is a keyword.
+if (!Tok->isOneOf(tok::identifier, tok::kw_class)) {
// If we hit any other kind of token, just bail.
return;
}
-// Memoize the attribute. (Note that 'class' is a legal attribute!)
-PropertyAttributes.push_back({Tok->TokenText, StringRef{}});
+const StringRef Attribute{Tok->TokenText};
+StringRef Value;
// Also handle `getter=getFoo` attributes.
// (Note: no check needed against `EndTok`, since its type is not
@@ -82,49 +83,66 @@ void
ObjCPropertyAttributeOrderFixer::sortPropertyAttributes(
return;
}
Tok = Tok->Next;
- PropertyAttributes.back().Value = Tok->TokenText;
+ Value = Tok->TokenText;
+}
+
+auto It = SortOrderMap.find(Attribute);
+if (It == SortOrderMap.end())
+ It = SortOrderMap.insert({Attribute, SortOrderMap.size()}).first;
+
+// Sort the indices based on the priority stored in 'SortOrderMap'.
+const auto Ordinal = It->second;
+if (!Ordinals.insert(Ordinal).second) {
+ HasDuplicates = true;
+ continue;
}
+
+if (Ordinal >= Indices.size())
+ Indices.resize(Ordinal + 1);
+Indices[Ordinal] = Index++;
+
+// Memoize the attribute.
+PropertyAttributes.push_back({Attribute, Value});
}
- // There's nothing to do unless there's more than one attribute.
- if (PropertyAttributes.size() < 2)
-return;
+ if (!HasDuplicates) {
+// There's nothing to do unless there's more than one attribute.
+if (PropertyAttributes.size() < 2)
+ return;
- // Create a "remapping index" on how to reorder the attributes.
- SmallVector Indices =
- llvm::to_vector<8>(llvm::seq(0, PropertyAttributes.size()));
-
- // Sort the indices based on the priority stored in 'SortOrderMap'; use Max
- // for missing values.
- const auto SortOrderMax = Style.ObjCPropertyAttributeOrder.size();
- auto SortIndex = [&](const StringRef ) -> unsigned {
-auto I = SortOrderMap.find(Needle);
-return (I == SortOrderMap.end()) ? SortOrderMax : I->getValue();
- };
- llvm::stable_sort(Indices, [&](unsigned LHSI,
[clang] [clang-format] Remove duplicates in @property using std::set (PR #74235)
https://github.com/owenca created
https://github.com/llvm/llvm-project/pull/74235
Re-implement ObjCPropertyAttributeOrder using std::set for sorting and removing
duplicates. (We can't use llvm::SmallSet because it's unordered.)
>From 7323d9261fe8c876fe3a656a98e186af3dd0b2a0 Mon Sep 17 00:00:00 2001
From: Owen Pan
Date: Sun, 3 Dec 2023 01:41:57 -0800
Subject: [PATCH] [clang-format] Remove duplicates in @property using std::set
Re-implement ObjCPropertyAttributeOrder using std::set for sorting and
removing duplicates. (We can't use llvm::SmallSet because it's unordered.)
---
.../ObjCPropertyAttributeOrderFixer.cpp | 116 ++
.../Format/ObjCPropertyAttributeOrderFixer.h | 4 +-
.../ObjCPropertyAttributeOrderFixerTest.cpp | 12 +-
3 files changed, 75 insertions(+), 57 deletions(-)
diff --git a/clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp
b/clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp
index 20108306f1039..57b19a62154d1 100644
--- a/clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp
+++ b/clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp
@@ -15,8 +15,6 @@
#include "ObjCPropertyAttributeOrderFixer.h"
-#include "llvm/ADT/Sequence.h"
-
#include
namespace clang {
@@ -25,11 +23,10 @@ namespace format {
ObjCPropertyAttributeOrderFixer::ObjCPropertyAttributeOrderFixer(
const Environment , const FormatStyle )
: TokenAnalyzer(Env, Style) {
-
// Create an "order priority" map to use to sort properties.
- unsigned index = 0;
+ unsigned Index = 0;
for (const auto : Style.ObjCPropertyAttributeOrder)
-SortOrderMap[Property] = index++;
+SortOrderMap[Property] = Index++;
}
struct ObjCPropertyEntry {
@@ -37,14 +34,9 @@ struct ObjCPropertyEntry {
StringRef Value; // eg, the "foo" of the attribute "getter=foo"
};
-static bool isObjCPropertyAttribute(const FormatToken *Tok) {
- // Most attributes look like identifiers, but `class` is a keyword.
- return Tok->isOneOf(tok::identifier, tok::kw_class);
-}
-
void ObjCPropertyAttributeOrderFixer::sortPropertyAttributes(
const SourceManager , tooling::Replacements ,
-const FormatToken *BeginTok, const FormatToken *EndTok) const {
+const FormatToken *BeginTok, const FormatToken *EndTok) {
assert(BeginTok);
assert(EndTok);
assert(EndTok->Previous);
@@ -53,8 +45,16 @@ void ObjCPropertyAttributeOrderFixer::sortPropertyAttributes(
if (BeginTok == EndTok || BeginTok->Next == EndTok)
return;
+ // Use a set to sort attributes and remove duplicates.
+ std::set Ordinals;
+
+ // Create a "remapping index" on how to reorder the attributes.
+ SmallVector Indices;
+
// Collect the attributes.
- SmallVector PropertyAttributes;
+ SmallVector PropertyAttributes;
+ bool HasDuplicates = false;
+ int Index = 0;
for (auto Tok = BeginTok; Tok != EndTok; Tok = Tok->Next) {
assert(Tok);
if (Tok->is(tok::comma)) {
@@ -62,13 +62,14 @@ void
ObjCPropertyAttributeOrderFixer::sortPropertyAttributes(
continue;
}
-if (!isObjCPropertyAttribute(Tok)) {
+// Most attributes look like identifiers, but `class` is a keyword.
+if (!Tok->isOneOf(tok::identifier, tok::kw_class)) {
// If we hit any other kind of token, just bail.
return;
}
-// Memoize the attribute. (Note that 'class' is a legal attribute!)
-PropertyAttributes.push_back({Tok->TokenText, StringRef{}});
+const StringRef Attribute{Tok->TokenText};
+StringRef Value;
// Also handle `getter=getFoo` attributes.
// (Note: no check needed against `EndTok`, since its type is not
@@ -82,49 +83,66 @@ void
ObjCPropertyAttributeOrderFixer::sortPropertyAttributes(
return;
}
Tok = Tok->Next;
- PropertyAttributes.back().Value = Tok->TokenText;
+ Value = Tok->TokenText;
+}
+
+auto It = SortOrderMap.find(Attribute);
+if (It == SortOrderMap.end())
+ It = SortOrderMap.insert({Attribute, SortOrderMap.size()}).first;
+
+// Sort the indices based on the priority stored in 'SortOrderMap'.
+const auto Ordinal = It->second;
+if (!Ordinals.insert(Ordinal).second) {
+ HasDuplicates = true;
+ continue;
}
+
+if (Ordinal >= Indices.size())
+ Indices.resize(Ordinal + 1);
+Indices[Ordinal] = Index++;
+
+// Memoize the attribute.
+PropertyAttributes.push_back({Attribute, Value});
}
- // There's nothing to do unless there's more than one attribute.
- if (PropertyAttributes.size() < 2)
-return;
+ if (!HasDuplicates) {
+// There's nothing to do unless there's more than one attribute.
+if (PropertyAttributes.size() < 2)
+ return;
- // Create a "remapping index" on how to reorder the attributes.
- SmallVector Indices =
- llvm::to_vector<8>(llvm::seq(0, PropertyAttributes.size()));
-
- // Sort the indices based on the priority stored in 'SortOrderMap'; use Max
- // for missing values.
- const auto SortOrderMax =
