[PATCH] D25647: [clang-tidy] Don't use a SmallSetVector of an enum.
jlebar added a comment. Thank you for the reviews, everyone! Repository: rL LLVM https://reviews.llvm.org/D25647 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25647: [clang-tidy] Don't use a SmallSetVector of an enum.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL284873: [clang-tidy] Don't use a SmallSetVector of an enum.
(authored by jlebar).
Changed prior to commit:
https://reviews.llvm.org/D25647?vs=75443=75478#toc
Repository:
rL LLVM
https://reviews.llvm.org/D25647
Files:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
Index:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
===
---
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
+++
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
@@ -31,7 +31,7 @@
void check(const ast_matchers::MatchFinder::MatchResult ) override;
void onEndOfTranslationUnit() override;
- enum class SpecialMemberFunctionKind {
+ enum class SpecialMemberFunctionKind : uint8_t {
Destructor,
CopyConstructor,
CopyAssignment,
@@ -43,7 +43,7 @@
using ClassDefiningSpecialMembersMap =
llvm::DenseMap>;
+ llvm::SmallVector>;
private:
ClassDefiningSpecialMembersMap ClassWithSpecialMembers;
Index:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
===
---
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
+++
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
@@ -97,8 +97,13 @@
{"move-assign", SpecialMemberFunctionKind::MoveAssignment}};
for (const auto : Matchers)
-if (Result.Nodes.getNodeAs(KV.first))
- ClassWithSpecialMembers[ID].insert(KV.second);
+if (Result.Nodes.getNodeAs(KV.first)) {
+ SpecialMemberFunctionKind Kind = KV.second;
+ llvm::SmallVectorImpl =
+ ClassWithSpecialMembers[ID];
+ if (find(Members, Kind) == Members.end())
+Members.push_back(Kind);
+}
}
void SpecialMemberFunctionsCheck::onEndOfTranslationUnit() {
@@ -125,7 +130,7 @@
std::back_inserter(UndefinedSpecialMembers));
diag(C.first.first, "class '%0' defines %1 but does not define %2")
-<< C.first.second << join(DefinedSpecialMembers.getArrayRef(), " and ")
+<< C.first.second << join(DefinedSpecialMembers, " and ")
<< join(UndefinedSpecialMembers, " or ");
}
}
Index: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
@@ -31,7 +31,7 @@
void check(const ast_matchers::MatchFinder::MatchResult ) override;
void onEndOfTranslationUnit() override;
- enum class SpecialMemberFunctionKind {
+ enum class SpecialMemberFunctionKind : uint8_t {
Destructor,
CopyConstructor,
CopyAssignment,
@@ -43,7 +43,7 @@
using ClassDefiningSpecialMembersMap =
llvm::DenseMap>;
+ llvm::SmallVector>;
private:
ClassDefiningSpecialMembersMap ClassWithSpecialMembers;
Index: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
@@ -97,8 +97,13 @@
{"move-assign", SpecialMemberFunctionKind::MoveAssignment}};
for (const auto : Matchers)
-if (Result.Nodes.getNodeAs(KV.first))
- ClassWithSpecialMembers[ID].insert(KV.second);
+if (Result.Nodes.getNodeAs(KV.first)) {
+ SpecialMemberFunctionKind Kind = KV.second;
+ llvm::SmallVectorImpl =
+ ClassWithSpecialMembers[ID];
+ if (find(Members, Kind) == Members.end())
+Members.push_back(Kind);
+}
}
void SpecialMemberFunctionsCheck::onEndOfTranslationUnit() {
@@ -125,7 +130,7 @@
std::back_inserter(UndefinedSpecialMembers));
diag(C.first.first, "class '%0' defines %1 but does not define %2")
-<< C.first.second << join(DefinedSpecialMembers.getArrayRef(), " and ")
+<< C.first.second << join(DefinedSpecialMembers, " and ")
<< join(UndefinedSpecialMembers, " or ");
}
}
___
cfe-commits mailing list
[PATCH] D25647: [clang-tidy] Don't use a SmallSetVector of an enum.
jlebar added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:103 + auto = ClassWithSpecialMembers[ID]; + if (find(Members, Kind) == Members.end()) Members.push_back(Kind); +} jlebar wrote: > bkramer wrote: > > jlebar wrote: > > > aaron.ballman wrote: > > > > Please drop the `push_back()` onto its own line. > > > This was actually how clang-tidy formatted the line. My understanding is > > > that we go with that in llvm projects? Unless you're saying I have an > > > outdated clang-tidy, in which case I will investigate that. > > It's what happens when your clang-format is using Google style. Some > > versions of clang-format ship with weird defaults, so you have to add > > -style=LLVM manually. > Er, s/clang-tidy/clang-format/ Oh, this is happening because clang-tools-extra does not have a .clang-format file in tree. It's relying on us picking up clang's .clang-format file, but if you use the monorepo [0] (or if you have a tool that's smart and stops at git repository boundaries) you won't find it. Mystery solved. I can check those files in if you want, or not if you want. :) [0] https://github.com/llvm-project/llvm-project https://reviews.llvm.org/D25647 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25647: [clang-tidy] Don't use a SmallSetVector of an enum.
jlebar updated this revision to Diff 75443.
jlebar marked 5 inline comments as done.
jlebar added a comment.
Adjust formatting, write out type of 'auto'.
https://reviews.llvm.org/D25647
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
Index:
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
@@ -31,7 +31,7 @@
void check(const ast_matchers::MatchFinder::MatchResult ) override;
void onEndOfTranslationUnit() override;
- enum class SpecialMemberFunctionKind {
+ enum class SpecialMemberFunctionKind : uint8_t {
Destructor,
CopyConstructor,
CopyAssignment,
@@ -43,7 +43,7 @@
using ClassDefiningSpecialMembersMap =
llvm::DenseMap>;
+ llvm::SmallVector>;
private:
ClassDefiningSpecialMembersMap ClassWithSpecialMembers;
Index:
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
===
---
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
+++
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
@@ -97,8 +97,13 @@
{"move-assign", SpecialMemberFunctionKind::MoveAssignment}};
for (const auto : Matchers)
-if (Result.Nodes.getNodeAs(KV.first))
- ClassWithSpecialMembers[ID].insert(KV.second);
+if (Result.Nodes.getNodeAs(KV.first)) {
+ SpecialMemberFunctionKind Kind = KV.second;
+ llvm::SmallVectorImpl =
+ ClassWithSpecialMembers[ID];
+ if (find(Members, Kind) == Members.end())
+Members.push_back(Kind);
+}
}
void SpecialMemberFunctionsCheck::onEndOfTranslationUnit() {
@@ -125,7 +130,7 @@
std::back_inserter(UndefinedSpecialMembers));
diag(C.first.first, "class '%0' defines %1 but does not define %2")
-<< C.first.second << join(DefinedSpecialMembers.getArrayRef(), " and ")
+<< C.first.second << join(DefinedSpecialMembers, " and ")
<< join(UndefinedSpecialMembers, " or ");
}
}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
@@ -31,7 +31,7 @@
void check(const ast_matchers::MatchFinder::MatchResult ) override;
void onEndOfTranslationUnit() override;
- enum class SpecialMemberFunctionKind {
+ enum class SpecialMemberFunctionKind : uint8_t {
Destructor,
CopyConstructor,
CopyAssignment,
@@ -43,7 +43,7 @@
using ClassDefiningSpecialMembersMap =
llvm::DenseMap>;
+ llvm::SmallVector>;
private:
ClassDefiningSpecialMembersMap ClassWithSpecialMembers;
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
@@ -97,8 +97,13 @@
{"move-assign", SpecialMemberFunctionKind::MoveAssignment}};
for (const auto : Matchers)
-if (Result.Nodes.getNodeAs(KV.first))
- ClassWithSpecialMembers[ID].insert(KV.second);
+if (Result.Nodes.getNodeAs(KV.first)) {
+ SpecialMemberFunctionKind Kind = KV.second;
+ llvm::SmallVectorImpl =
+ ClassWithSpecialMembers[ID];
+ if (find(Members, Kind) == Members.end())
+Members.push_back(Kind);
+}
}
void SpecialMemberFunctionsCheck::onEndOfTranslationUnit() {
@@ -125,7 +130,7 @@
std::back_inserter(UndefinedSpecialMembers));
diag(C.first.first, "class '%0' defines %1 but does not define %2")
-<< C.first.second << join(DefinedSpecialMembers.getArrayRef(), " and ")
+<< C.first.second << join(DefinedSpecialMembers, " and ")
<< join(UndefinedSpecialMembers, " or ");
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25647: [clang-tidy] Don't use a SmallSetVector of an enum.
jlebar added a comment. Thank you very much for the review! Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:103 + auto = ClassWithSpecialMembers[ID]; + if (find(Members, Kind) == Members.end()) Members.push_back(Kind); +} aaron.ballman wrote: > Please drop the `push_back()` onto its own line. This was actually how clang-tidy formatted the line. My understanding is that we go with that in llvm projects? Unless you're saying I have an outdated clang-tidy, in which case I will investigate that. Repository: rL LLVM https://reviews.llvm.org/D25647 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25647: [clang-tidy] Don't use a SmallSetVector of an enum.
jlebar added a comment. Thank you very much for the review! Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:103 + auto = ClassWithSpecialMembers[ID]; + if (find(Members, Kind) == Members.end()) Members.push_back(Kind); +} bkramer wrote: > jlebar wrote: > > aaron.ballman wrote: > > > Please drop the `push_back()` onto its own line. > > This was actually how clang-tidy formatted the line. My understanding is > > that we go with that in llvm projects? Unless you're saying I have an > > outdated clang-tidy, in which case I will investigate that. > It's what happens when your clang-format is using Google style. Some versions > of clang-format ship with weird defaults, so you have to add -style=LLVM > manually. Er, s/clang-tidy/clang-format/ Repository: rL LLVM https://reviews.llvm.org/D25647 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25647: [clang-tidy] Don't use a SmallSetVector of an enum.
bkramer added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:103 + auto = ClassWithSpecialMembers[ID]; + if (find(Members, Kind) == Members.end()) Members.push_back(Kind); +} jlebar wrote: > aaron.ballman wrote: > > Please drop the `push_back()` onto its own line. > This was actually how clang-tidy formatted the line. My understanding is > that we go with that in llvm projects? Unless you're saying I have an > outdated clang-tidy, in which case I will investigate that. It's what happens when your clang-format is using Google style. Some versions of clang-format ship with weird defaults, so you have to add -style=LLVM manually. Repository: rL LLVM https://reviews.llvm.org/D25647 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25647: [clang-tidy] Don't use a SmallSetVector of an enum.
jlebar added a comment. Hi, friendly ping? This trivial patch is the only blocker remaining before I can land https://reviews.llvm.org/D25648, which is the first part of my Grand Set Refactoring (see mail to llvm-dev about a week ago). Repository: rL LLVM https://reviews.llvm.org/D25647 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25647: [clang-tidy] Don't use a SmallSetVector of an enum.
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with two minor nits. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:102 + SpecialMemberFunctionKind Kind = KV.second; + auto = ClassWithSpecialMembers[ID]; + if (find(Members, Kind) == Members.end()) Members.push_back(Kind); Please don't use `auto` since the type is not spelled out in the initializer. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:103 + auto = ClassWithSpecialMembers[ID]; + if (find(Members, Kind) == Members.end()) Members.push_back(Kind); +} Please drop the `push_back()` onto its own line. Repository: rL LLVM https://reviews.llvm.org/D25647 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25647: [clang-tidy] Don't use a SmallSetVector of an enum.
jlebar created this revision.
jlebar added a reviewer: timshen.
jlebar added a subscriber: cfe-commits.
Herald added a subscriber: nemanjai.
This doesn't work after converting SmallSetVector to use DenseSet.
Instead we can just use a SmallVector.
https://reviews.llvm.org/D25647
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
Index:
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
@@ -31,7 +31,7 @@
void check(const ast_matchers::MatchFinder::MatchResult ) override;
void onEndOfTranslationUnit() override;
- enum class SpecialMemberFunctionKind {
+ enum class SpecialMemberFunctionKind : uint8_t {
Destructor,
CopyConstructor,
CopyAssignment,
@@ -43,9 +43,9 @@
using ClassDefiningSpecialMembersMap =
llvm::DenseMap>;
+ llvm::SmallVector>;
-private:
+ private:
ClassDefiningSpecialMembersMap ClassWithSpecialMembers;
};
Index:
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
===
---
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
+++
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
@@ -97,8 +97,11 @@
{"move-assign", SpecialMemberFunctionKind::MoveAssignment}};
for (const auto : Matchers)
-if (Result.Nodes.getNodeAs(KV.first))
- ClassWithSpecialMembers[ID].insert(KV.second);
+if (Result.Nodes.getNodeAs(KV.first)) {
+ SpecialMemberFunctionKind Kind = KV.second;
+ auto = ClassWithSpecialMembers[ID];
+ if (find(Members, Kind) == Members.end()) Members.push_back(Kind);
+}
}
void SpecialMemberFunctionsCheck::onEndOfTranslationUnit() {
@@ -125,7 +128,7 @@
std::back_inserter(UndefinedSpecialMembers));
diag(C.first.first, "class '%0' defines %1 but does not define %2")
-<< C.first.second << join(DefinedSpecialMembers.getArrayRef(), " and ")
+<< C.first.second << join(DefinedSpecialMembers, " and ")
<< join(UndefinedSpecialMembers, " or ");
}
}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
@@ -31,7 +31,7 @@
void check(const ast_matchers::MatchFinder::MatchResult ) override;
void onEndOfTranslationUnit() override;
- enum class SpecialMemberFunctionKind {
+ enum class SpecialMemberFunctionKind : uint8_t {
Destructor,
CopyConstructor,
CopyAssignment,
@@ -43,9 +43,9 @@
using ClassDefiningSpecialMembersMap =
llvm::DenseMap>;
+ llvm::SmallVector>;
-private:
+ private:
ClassDefiningSpecialMembersMap ClassWithSpecialMembers;
};
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
@@ -97,8 +97,11 @@
{"move-assign", SpecialMemberFunctionKind::MoveAssignment}};
for (const auto : Matchers)
-if (Result.Nodes.getNodeAs(KV.first))
- ClassWithSpecialMembers[ID].insert(KV.second);
+if (Result.Nodes.getNodeAs(KV.first)) {
+ SpecialMemberFunctionKind Kind = KV.second;
+ auto = ClassWithSpecialMembers[ID];
+ if (find(Members, Kind) == Members.end()) Members.push_back(Kind);
+}
}
void SpecialMemberFunctionsCheck::onEndOfTranslationUnit() {
@@ -125,7 +128,7 @@
std::back_inserter(UndefinedSpecialMembers));
diag(C.first.first, "class '%0' defines %1 but does not define %2")
-<< C.first.second << join(DefinedSpecialMembers.getArrayRef(), " and ")
+<< C.first.second << join(DefinedSpecialMembers, " and ")
<< join(UndefinedSpecialMembers, " or ");
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
