Re: [PATCH] D82225: [libTooling] Delete deprecated `Stencil` combinators.
On Thu, Jun 25, 2020 at 5:53 AM Yitzhak Mandelbaum wrote:
>
> Thanks for alerting me. Why is it important to strip out this information?
> Also, might this be something we can change on the arcanist side (to not add
> to the commit message) rather than strip out on the git end?
Mostly to keep commit descriptions tidy. In a paper of git log output,
large volume of unneeded Phabricator tags dilutes useful information.
It is said that hacking on the Phabricator side can prevent pushing
some tags, but it may also lose convenience because currently one can
edit Reviewers: and Subscribers:.
> On Wed, Jun 24, 2020 at 3:28 PM Fangrui Song via Phabricator
> wrote:
>>
>> MaskRay added a comment.
>>
>> Hi, your git commit contains extra Phabricator tags. You can drop
>> `Reviewers:` `Subscribers:` `Tags:` and the text `Summary:` from the git
>> commit with the following script:
>>
>> arcfilter () {
>> arc amend
>> git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1}
>> /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/
>> {sub(/^Summary: /,"");print}' | git commit --amend --date=now -F -
>> }
>>
>> `Reviewed By: ` is considered important by some people. Please keep the tag.
>> (I have updated my script to use `--date=now` (setting author date to
>> committer date))
>>
>> `https://reviews.llvm.org/D80978` contains a git pre-push hook to automate
>> this.
>>
>>
>> Repository:
>> rG LLVM Github Monorepo
>>
>> CHANGES SINCE LAST ACTION
>> https://reviews.llvm.org/D82225/new/
>>
>> https://reviews.llvm.org/D82225
>>
>>
>>
--
宋方睿
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D82225: [libTooling] Delete deprecated `Stencil` combinators.
Thanks for alerting me. Why is it important to strip out this information?
Also, might this be something we can change on the arcanist side (to not
add to the commit message) rather than strip out on the git end?
On Wed, Jun 24, 2020 at 3:28 PM Fangrui Song via Phabricator <
revi...@reviews.llvm.org> wrote:
> MaskRay added a comment.
>
> Hi, your git commit contains extra Phabricator tags. You can drop
> `Reviewers:` `Subscribers:` `Tags:` and the text `Summary:` from the git
> commit with the following script:
>
> arcfilter () {
> arc amend
> git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1}
> /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/
> {sub(/^Summary: /,"");print}' | git commit --amend --date=now -F -
> }
>
> `Reviewed By: ` is considered important by some people. Please keep the
> tag. (I have updated my script to use `--date=now` (setting author date to
> committer date))
>
> `https://reviews.llvm.org/D80978`
> contains a git pre-push hook to automate this.
>
>
> Repository:
> rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
> https://reviews.llvm.org/D82225/new/
>
> https://reviews.llvm.org/D82225
>
>
>
>
smime.p7s
Description: S/MIME Cryptographic Signature
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D82225: [libTooling] Delete deprecated `Stencil` combinators.
MaskRay added a comment.
Hi, your git commit contains extra Phabricator tags. You can drop `Reviewers:`
`Subscribers:` `Tags:` and the text `Summary:` from the git commit with the
following script:
arcfilter () {
arc amend
git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1}
/Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary:
/,"");print}' | git commit --amend --date=now -F -
}
`Reviewed By: ` is considered important by some people. Please keep the tag. (I
have updated my script to use `--date=now` (setting author date to committer
date))
`https://reviews.llvm.org/D80978` contains a git pre-push hook to automate this.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82225/new/
https://reviews.llvm.org/D82225
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D82225: [libTooling] Delete deprecated `Stencil` combinators.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG87340a2bf1d2: [libTooling] Delete deprecated `Stencil`
combinators. (authored by ymandel).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82225/new/
https://reviews.llvm.org/D82225
Files:
clang/include/clang/Tooling/Transformer/Stencil.h
clang/lib/Tooling/Transformer/Stencil.cpp
clang/unittests/Tooling/StencilTest.cpp
Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -180,12 +180,12 @@
TEST_F(StencilTest, IfBoundOpBound) {
StringRef Id = "id";
- testExpr(Id, "3;", ifBound(Id, text("5"), text("7")), "5");
+ testExpr(Id, "3;", ifBound(Id, cat("5"), cat("7")), "5");
}
TEST_F(StencilTest, IfBoundOpUnbound) {
StringRef Id = "id";
- testExpr(Id, "3;", ifBound("other", text("5"), text("7")), "7");
+ testExpr(Id, "3;", ifBound("other", cat("5"), cat("7")), "7");
}
TEST_F(StencilTest, ExpressionOpNoParens) {
@@ -293,7 +293,7 @@
x;
)cc";
StringRef Id = "id";
- testExpr(Id, Snippet, access(Id, text("field")), "x.field");
+ testExpr(Id, Snippet, access(Id, cat("field")), "x.field");
}
TEST_F(StencilTest, AccessOpValueAddress) {
@@ -479,7 +479,7 @@
}
TEST(StencilToStringTest, AccessOpSelector) {
- auto S = access("Id", selection(name("otherId")));
+ auto S = access("Id", cat(name("otherId")));
StringRef Expected = R"repr(access("Id", selection(...)))repr";
EXPECT_EQ(S->toString(), Expected);
}
@@ -491,7 +491,7 @@
}
TEST(StencilToStringTest, IfBoundOp) {
- auto S = ifBound("Id", text("trueText"), access("exprId", "memberData"));
+ auto S = ifBound("Id", cat("trueText"), access("exprId", "memberData"));
StringRef Expected =
R"repr(ifBound("Id", "trueText", access("exprId", "memberData")))repr";
EXPECT_EQ(S->toString(), Expected);
@@ -505,7 +505,7 @@
TEST(StencilToStringTest, Sequence) {
auto S = cat("foo", access("x", "m()"), "bar",
- ifBound("x", text("t"), access("e", "f")));
+ ifBound("x", cat("t"), access("e", "f")));
StringRef Expected = R"repr(seq("foo", access("x", "m()"), "bar", )repr"
R"repr(ifBound("x", "t", access("e", "f"repr";
EXPECT_EQ(S->toString(), Expected);
@@ -524,8 +524,8 @@
}
TEST(StencilToStringTest, SequenceFromVector) {
- auto S = catVector({text("foo"), access("x", "m()"), text("bar"),
- ifBound("x", text("t"), access("e", "f"))});
+ auto S = catVector({cat("foo"), access("x", "m()"), cat("bar"),
+ ifBound("x", cat("t"), access("e", "f"))});
StringRef Expected = R"repr(seq("foo", access("x", "m()"), "bar", )repr"
R"repr(ifBound("x", "t", access("e", "f"repr";
EXPECT_EQ(S->toString(), Expected);
Index: clang/lib/Tooling/Transformer/Stencil.cpp
===
--- clang/lib/Tooling/Transformer/Stencil.cpp
+++ clang/lib/Tooling/Transformer/Stencil.cpp
@@ -298,17 +298,11 @@
};
} // namespace
-Stencil transformer::detail::makeStencil(StringRef Text) { return text(Text); }
-
-Stencil transformer::detail::makeStencil(RangeSelector Selector) {
- return selection(std::move(Selector));
-}
-
-Stencil transformer::text(StringRef Text) {
+Stencil transformer::detail::makeStencil(StringRef Text) {
return std::make_shared>(std::string(Text));
}
-Stencil transformer::selection(RangeSelector Selector) {
+Stencil transformer::detail::makeStencil(RangeSelector Selector) {
return std::make_shared>(std::move(Selector));
}
Index: clang/include/clang/Tooling/Transformer/Stencil.h
===
--- clang/include/clang/Tooling/Transformer/Stencil.h
+++ clang/include/clang/Tooling/Transformer/Stencil.h
@@ -69,14 +69,6 @@
// Functions for conveniently building stencils.
//
-/// DEPRECATED: Use `cat` instead.
-/// \returns exactly the text provided.
-Stencil text(llvm::StringRef Text);
-
-/// DEPRECATED: Use `cat` instead.
-/// \returns the source corresponding to the selected range.
-Stencil selection(RangeSelector Selector);
-
/// Generates the source of the expression bound to \p Id, wrapping it in
/// parentheses if it may parse differently depending on context. For example, a
/// binary operation is always wrapped, while a variable reference is never
@@ -112,7 +104,7 @@
/// Additionally, `e` is wrapped in parentheses, if needed.
Stencil access(llvm::StringRef BaseId, Stencil Member);
inline Stencil access(llvm::StringRef BaseId, llvm::StringRef Member) {
- return access(BaseId, text(Member));
+ return access(BaseId, detail::makeStencil(Member));
}
/// Chooses between the two stencil parts, based on whether \p ID is bound in
@@ -123,7 +115,8 @@
/// match.
inline
[PATCH] D82225: [libTooling] Delete deprecated `Stencil` combinators.
ymandel updated this revision to Diff 272157.
ymandel added a comment.
fix additional refs
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82225/new/
https://reviews.llvm.org/D82225
Files:
clang/include/clang/Tooling/Transformer/Stencil.h
clang/lib/Tooling/Transformer/Stencil.cpp
clang/unittests/Tooling/StencilTest.cpp
Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -179,12 +179,12 @@
TEST_F(StencilTest, IfBoundOpBound) {
StringRef Id = "id";
- testExpr(Id, "3;", ifBound(Id, text("5"), text("7")), "5");
+ testExpr(Id, "3;", ifBound(Id, cat("5"), cat("7")), "5");
}
TEST_F(StencilTest, IfBoundOpUnbound) {
StringRef Id = "id";
- testExpr(Id, "3;", ifBound("other", text("5"), text("7")), "7");
+ testExpr(Id, "3;", ifBound("other", cat("5"), cat("7")), "7");
}
TEST_F(StencilTest, ExpressionOpNoParens) {
@@ -292,7 +292,7 @@
x;
)cc";
StringRef Id = "id";
- testExpr(Id, Snippet, access(Id, text("field")), "x.field");
+ testExpr(Id, Snippet, access(Id, cat("field")), "x.field");
}
TEST_F(StencilTest, AccessOpValueAddress) {
@@ -436,7 +436,7 @@
}
TEST(StencilToStringTest, AccessOpSelector) {
- auto S = access("Id", selection(name("otherId")));
+ auto S = access("Id", cat(name("otherId")));
StringRef Expected = R"repr(access("Id", selection(...)))repr";
EXPECT_EQ(S->toString(), Expected);
}
@@ -448,7 +448,7 @@
}
TEST(StencilToStringTest, IfBoundOp) {
- auto S = ifBound("Id", text("trueText"), access("exprId", "memberData"));
+ auto S = ifBound("Id", cat("trueText"), access("exprId", "memberData"));
StringRef Expected =
R"repr(ifBound("Id", "trueText", access("exprId", "memberData")))repr";
EXPECT_EQ(S->toString(), Expected);
@@ -462,7 +462,7 @@
TEST(StencilToStringTest, Sequence) {
auto S = cat("foo", access("x", "m()"), "bar",
- ifBound("x", text("t"), access("e", "f")));
+ ifBound("x", cat("t"), access("e", "f")));
StringRef Expected = R"repr(seq("foo", access("x", "m()"), "bar", )repr"
R"repr(ifBound("x", "t", access("e", "f"repr";
EXPECT_EQ(S->toString(), Expected);
@@ -481,8 +481,8 @@
}
TEST(StencilToStringTest, SequenceFromVector) {
- auto S = catVector({text("foo"), access("x", "m()"), text("bar"),
- ifBound("x", text("t"), access("e", "f"))});
+ auto S = catVector({cat("foo"), access("x", "m()"), cat("bar"),
+ ifBound("x", cat("t"), access("e", "f"))});
StringRef Expected = R"repr(seq("foo", access("x", "m()"), "bar", )repr"
R"repr(ifBound("x", "t", access("e", "f"repr";
EXPECT_EQ(S->toString(), Expected);
Index: clang/lib/Tooling/Transformer/Stencil.cpp
===
--- clang/lib/Tooling/Transformer/Stencil.cpp
+++ clang/lib/Tooling/Transformer/Stencil.cpp
@@ -295,17 +295,11 @@
};
} // namespace
-Stencil transformer::detail::makeStencil(StringRef Text) { return text(Text); }
-
-Stencil transformer::detail::makeStencil(RangeSelector Selector) {
- return selection(std::move(Selector));
-}
-
-Stencil transformer::text(StringRef Text) {
+Stencil transformer::detail::makeStencil(StringRef Text) {
return std::make_shared>(std::string(Text));
}
-Stencil transformer::selection(RangeSelector Selector) {
+Stencil transformer::detail::makeStencil(RangeSelector Selector) {
return std::make_shared>(std::move(Selector));
}
Index: clang/include/clang/Tooling/Transformer/Stencil.h
===
--- clang/include/clang/Tooling/Transformer/Stencil.h
+++ clang/include/clang/Tooling/Transformer/Stencil.h
@@ -69,14 +69,6 @@
// Functions for conveniently building stencils.
//
-/// DEPRECATED: Use `cat` instead.
-/// \returns exactly the text provided.
-Stencil text(llvm::StringRef Text);
-
-/// DEPRECATED: Use `cat` instead.
-/// \returns the source corresponding to the selected range.
-Stencil selection(RangeSelector Selector);
-
/// Generates the source of the expression bound to \p Id, wrapping it in
/// parentheses if it may parse differently depending on context. For example, a
/// binary operation is always wrapped, while a variable reference is never
@@ -112,7 +104,7 @@
/// Additionally, `e` is wrapped in parentheses, if needed.
Stencil access(llvm::StringRef BaseId, Stencil Member);
inline Stencil access(llvm::StringRef BaseId, llvm::StringRef Member) {
- return access(BaseId, text(Member));
+ return access(BaseId, detail::makeStencil(Member));
}
/// Chooses between the two stencil parts, based on whether \p ID is bound in
@@ -123,7 +115,8 @@
/// match.
inline Stencil ifBound(llvm::StringRef Id, llvm::StringRef TrueText,
[PATCH] D82225: [libTooling] Delete deprecated `Stencil` combinators.
ymandel created this revision.
ymandel added a reviewer: tdl-g.
Herald added a project: clang.
Deletes `text()` and `selection()` combinators, since they have been deprecated
for months.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D82225
Files:
clang/include/clang/Tooling/Transformer/Stencil.h
clang/lib/Tooling/Transformer/Stencil.cpp
clang/unittests/Tooling/StencilTest.cpp
Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -179,12 +179,12 @@
TEST_F(StencilTest, IfBoundOpBound) {
StringRef Id = "id";
- testExpr(Id, "3;", ifBound(Id, text("5"), text("7")), "5");
+ testExpr(Id, "3;", ifBound(Id, cat("5"), cat("7")), "5");
}
TEST_F(StencilTest, IfBoundOpUnbound) {
StringRef Id = "id";
- testExpr(Id, "3;", ifBound("other", text("5"), text("7")), "7");
+ testExpr(Id, "3;", ifBound("other", cat("5"), cat("7")), "7");
}
TEST_F(StencilTest, ExpressionOpNoParens) {
@@ -292,7 +292,7 @@
x;
)cc";
StringRef Id = "id";
- testExpr(Id, Snippet, access(Id, text("field")), "x.field");
+ testExpr(Id, Snippet, access(Id, cat("field")), "x.field");
}
TEST_F(StencilTest, AccessOpValueAddress) {
@@ -448,7 +448,7 @@
}
TEST(StencilToStringTest, IfBoundOp) {
- auto S = ifBound("Id", text("trueText"), access("exprId", "memberData"));
+ auto S = ifBound("Id", cat("trueText"), access("exprId", "memberData"));
StringRef Expected =
R"repr(ifBound("Id", "trueText", access("exprId", "memberData")))repr";
EXPECT_EQ(S->toString(), Expected);
@@ -462,7 +462,7 @@
TEST(StencilToStringTest, Sequence) {
auto S = cat("foo", access("x", "m()"), "bar",
- ifBound("x", text("t"), access("e", "f")));
+ ifBound("x", cat("t"), access("e", "f")));
StringRef Expected = R"repr(seq("foo", access("x", "m()"), "bar", )repr"
R"repr(ifBound("x", "t", access("e", "f"repr";
EXPECT_EQ(S->toString(), Expected);
@@ -481,8 +481,8 @@
}
TEST(StencilToStringTest, SequenceFromVector) {
- auto S = catVector({text("foo"), access("x", "m()"), text("bar"),
- ifBound("x", text("t"), access("e", "f"))});
+ auto S = catVector({cat("foo"), access("x", "m()"), cat("bar"),
+ ifBound("x", cat("t"), access("e", "f"))});
StringRef Expected = R"repr(seq("foo", access("x", "m()"), "bar", )repr"
R"repr(ifBound("x", "t", access("e", "f"repr";
EXPECT_EQ(S->toString(), Expected);
Index: clang/lib/Tooling/Transformer/Stencil.cpp
===
--- clang/lib/Tooling/Transformer/Stencil.cpp
+++ clang/lib/Tooling/Transformer/Stencil.cpp
@@ -295,17 +295,11 @@
};
} // namespace
-Stencil transformer::detail::makeStencil(StringRef Text) { return text(Text); }
-
-Stencil transformer::detail::makeStencil(RangeSelector Selector) {
- return selection(std::move(Selector));
-}
-
-Stencil transformer::text(StringRef Text) {
+Stencil transformer::detail::makeStencil(StringRef Text) {
return std::make_shared>(std::string(Text));
}
-Stencil transformer::selection(RangeSelector Selector) {
+Stencil transformer::detail::makeStencil(RangeSelector Selector) {
return std::make_shared>(std::move(Selector));
}
Index: clang/include/clang/Tooling/Transformer/Stencil.h
===
--- clang/include/clang/Tooling/Transformer/Stencil.h
+++ clang/include/clang/Tooling/Transformer/Stencil.h
@@ -69,14 +69,6 @@
// Functions for conveniently building stencils.
//
-/// DEPRECATED: Use `cat` instead.
-/// \returns exactly the text provided.
-Stencil text(llvm::StringRef Text);
-
-/// DEPRECATED: Use `cat` instead.
-/// \returns the source corresponding to the selected range.
-Stencil selection(RangeSelector Selector);
-
/// Generates the source of the expression bound to \p Id, wrapping it in
/// parentheses if it may parse differently depending on context. For example, a
/// binary operation is always wrapped, while a variable reference is never
@@ -112,7 +104,7 @@
/// Additionally, `e` is wrapped in parentheses, if needed.
Stencil access(llvm::StringRef BaseId, Stencil Member);
inline Stencil access(llvm::StringRef BaseId, llvm::StringRef Member) {
- return access(BaseId, text(Member));
+ return access(BaseId, detail::makeStencil(Member));
}
/// Chooses between the two stencil parts, based on whether \p ID is bound in
@@ -123,7 +115,8 @@
/// match.
inline Stencil ifBound(llvm::StringRef Id, llvm::StringRef TrueText,
llvm::StringRef FalseText) {
- return ifBound(Id, text(TrueText), text(FalseText));
+ return ifBound(Id, detail::makeStencil(TrueText),
+ detail::makeStencil(FalseText));
}
/// Wraps a \c MatchConsumer in a \c Stencil, so that it can be
