[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.
This revision was automatically updated to reflect the committed changes. Closed by commit rC358691: [LibTooling] Add Stencil library for format-string style codegen. (authored by ymandel, committed by ). Changed prior to commit: https://reviews.llvm.org/D59371?vs=195606&id=195778#toc Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 Files: include/clang/Tooling/Refactoring/Stencil.h lib/Tooling/Refactoring/CMakeLists.txt lib/Tooling/Refactoring/Stencil.cpp unittests/Tooling/CMakeLists.txt unittests/Tooling/StencilTest.cpp Index: unittests/Tooling/StencilTest.cpp === --- unittests/Tooling/StencilTest.cpp +++ unittests/Tooling/StencilTest.cpp @@ -0,0 +1,223 @@ +//===- unittest/Tooling/StencilTest.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "clang/Tooling/Refactoring/Stencil.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Tooling/FixIt.h" +#include "clang/Tooling/Tooling.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using namespace clang; +using namespace tooling; +using namespace ast_matchers; + +namespace { +using ::testing::AllOf; +using ::testing::Eq; +using ::testing::HasSubstr; +using MatchResult = MatchFinder::MatchResult; +using tooling::stencil::node; +using tooling::stencil::sNode; +using tooling::stencil::text; + +// In tests, we can't directly match on llvm::Expected since its accessors +// mutate the object. So, we collapse it to an Optional. +static llvm::Optional toOptional(llvm::Expected V) { + if (V) +return *V; + ADD_FAILURE() << "Losing error in conversion to IsSomething: " +<< llvm::toString(V.takeError()); + return llvm::None; +} + +// A very simple matcher for llvm::Optional values. +MATCHER_P(IsSomething, ValueMatcher, "") { + if (!arg) +return false; + return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener); +} + +// Create a valid translation-unit from a statement. +static std::string wrapSnippet(llvm::Twine StatementCode) { + return ("auto stencil_test_snippet = []{" + StatementCode + "};").str(); +} + +static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) { + return varDecl(hasName("stencil_test_snippet"), + hasDescendant(compoundStmt(hasAnySubstatement(Matcher; +} + +struct TestMatch { + // The AST unit from which `result` is built. We bundle it because it backs + // the result. Users are not expected to access it. + std::unique_ptr AstUnit; + // The result to use in the test. References `ast_unit`. + MatchResult Result; +}; + +// Matches `Matcher` against the statement `StatementCode` and returns the +// result. Handles putting the statement inside a function and modifying the +// matcher correspondingly. `Matcher` should match `StatementCode` exactly -- +// that is, produce exactly one match. +static llvm::Optional matchStmt(llvm::Twine StatementCode, + StatementMatcher Matcher) { + auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode)); + if (AstUnit == nullptr) { +ADD_FAILURE() << "AST construction failed"; +return llvm::None; + } + ASTContext &Context = AstUnit->getASTContext(); + auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context); + // We expect a single, exact match for the statement. + if (Matches.size() != 1) { +ADD_FAILURE() << "Wrong number of matches: " << Matches.size(); +return llvm::None; + } + return TestMatch{std::move(AstUnit), MatchResult(Matches[0], &Context)}; +} + +class StencilTest : public ::testing::Test { +protected: + // Verifies that the given stencil fails when evaluated on a valid match + // result. Binds a statement to "stmt", a (non-member) ctor-initializer to + // "init", an expression to "expr" and a (nameless) declaration to "decl". + void testError(const Stencil &Stencil, + ::testing::Matcher Matcher) { +const std::string Snippet = R"cc( + struct A {}; + class F : public A { + public: +F(int) {} + }; + F(1); +)cc"; +auto StmtMatch = matchStmt( +Snippet, +stmt(hasDescendant( + cxxConstructExpr( + hasDeclaration(decl(hasDescendant(cxxCtorInitializer( + isBaseInitializer()) + .bind("init"))) +.bind("decl"))) + .bind("expr"))) +.bind("stmt")); +ASSERT_TRUE(StmtMatch); +if (auto ResultOrEr
[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.
ymandel marked an inline comment as done. ymandel added inline comments. Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:78 + +template <> bool isEqualData(const RawTextData &A, const RawTextData &B) { + return A.Text == B.Text; ymandel wrote: > sbenza wrote: > > Any particular reason to use function template specialization instead of > > function overloading? > > The former is rare and thus harder to reason about. > > > > (same for `evalData` below) > Stylistic -- it forces you to explicitly declare the "overload set" so to > speak. However, i'm not particularly attached -- if you think this a bad > idea given its rarity, I'm happy to use overload sets. WDYT? Converted to normal overload sets (as per off-thread chat). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.
ymandel updated this revision to Diff 195606. ymandel added a comment. Convert template specialization to overload sets. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 Files: clang/include/clang/Tooling/Refactoring/Stencil.h clang/lib/Tooling/Refactoring/CMakeLists.txt clang/lib/Tooling/Refactoring/Stencil.cpp clang/unittests/Tooling/CMakeLists.txt clang/unittests/Tooling/StencilTest.cpp Index: clang/unittests/Tooling/StencilTest.cpp === --- /dev/null +++ clang/unittests/Tooling/StencilTest.cpp @@ -0,0 +1,223 @@ +//===- unittest/Tooling/StencilTest.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "clang/Tooling/Refactoring/Stencil.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Tooling/FixIt.h" +#include "clang/Tooling/Tooling.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using namespace clang; +using namespace tooling; +using namespace ast_matchers; + +namespace { +using ::testing::AllOf; +using ::testing::Eq; +using ::testing::HasSubstr; +using MatchResult = MatchFinder::MatchResult; +using tooling::stencil::node; +using tooling::stencil::sNode; +using tooling::stencil::text; + +// In tests, we can't directly match on llvm::Expected since its accessors +// mutate the object. So, we collapse it to an Optional. +static llvm::Optional toOptional(llvm::Expected V) { + if (V) +return *V; + ADD_FAILURE() << "Losing error in conversion to IsSomething: " +<< llvm::toString(V.takeError()); + return llvm::None; +} + +// A very simple matcher for llvm::Optional values. +MATCHER_P(IsSomething, ValueMatcher, "") { + if (!arg) +return false; + return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener); +} + +// Create a valid translation-unit from a statement. +static std::string wrapSnippet(llvm::Twine StatementCode) { + return ("auto stencil_test_snippet = []{" + StatementCode + "};").str(); +} + +static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) { + return varDecl(hasName("stencil_test_snippet"), + hasDescendant(compoundStmt(hasAnySubstatement(Matcher; +} + +struct TestMatch { + // The AST unit from which `result` is built. We bundle it because it backs + // the result. Users are not expected to access it. + std::unique_ptr AstUnit; + // The result to use in the test. References `ast_unit`. + MatchResult Result; +}; + +// Matches `Matcher` against the statement `StatementCode` and returns the +// result. Handles putting the statement inside a function and modifying the +// matcher correspondingly. `Matcher` should match `StatementCode` exactly -- +// that is, produce exactly one match. +static llvm::Optional matchStmt(llvm::Twine StatementCode, + StatementMatcher Matcher) { + auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode)); + if (AstUnit == nullptr) { +ADD_FAILURE() << "AST construction failed"; +return llvm::None; + } + ASTContext &Context = AstUnit->getASTContext(); + auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context); + // We expect a single, exact match for the statement. + if (Matches.size() != 1) { +ADD_FAILURE() << "Wrong number of matches: " << Matches.size(); +return llvm::None; + } + return TestMatch{std::move(AstUnit), MatchResult(Matches[0], &Context)}; +} + +class StencilTest : public ::testing::Test { +protected: + // Verifies that the given stencil fails when evaluated on a valid match + // result. Binds a statement to "stmt", a (non-member) ctor-initializer to + // "init", an expression to "expr" and a (nameless) declaration to "decl". + void testError(const Stencil &Stencil, + ::testing::Matcher Matcher) { +const std::string Snippet = R"cc( + struct A {}; + class F : public A { + public: +F(int) {} + }; + F(1); +)cc"; +auto StmtMatch = matchStmt( +Snippet, +stmt(hasDescendant( + cxxConstructExpr( + hasDeclaration(decl(hasDescendant(cxxCtorInitializer( + isBaseInitializer()) + .bind("init"))) +.bind("decl"))) + .bind("expr"))) +.bind("stmt")); +ASSERT_TRUE(StmtMatch); +if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) { + ADD_FAILURE() << "Expected failure but succeeded: " << *ResultOrErr; +} else { +
[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.
ymandel added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:127 + // Allow Stencils to operate as std::function, for compatibility with + // Transformer's TextGenerator. Calls `eval()` and asserts on failure. + std::string operator()(const ast_matchers::MatchFinder::MatchResult &) const; sbenza wrote: > ymandel wrote: > > sbenza wrote: > > > "asserts" as it only fails in debug mode? > > I thought `assert()` also fails in normal mode, based on my reading of the > > llvm docs, but it's not explicit about this: > > http://llvm.org/docs/ProgrammersManual.html#programmatic-errors > > > > FWIW, I'm on the fence whether `eval()` should actually have the same > > signature and similarly just crash the program on errors. Its not clear > > there's any value to propogating the errors up the stack via `Expected`. > `assert` follows `NDEBUG`, which is not explicitly related to `-Ox`, but they > usually go together. > > Don't make it crash, please. > Otherwise it would be hard to have a refactoring service or alike. Right, you've convinced me. For some background: TextGenerator (in Transformer) originally return an Expected, but Ilya suggested that I simplify to just a string and use assertions. The logic being that these are compiler-style tools. However, I agree that this simply doesn't work for running these in servers, which is a very real concern. It's also trivial to imagine a user mistyping a stencil (say, in web UI) which results in a failure in stencil eval. We want to return a nice error to the user, not crash the server. So, I've adjusted the signature. I will (separately) change the definition of TextGenerator in Transformer. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:101 +S.Parts.reserve(sizeof...(Parts)); +auto Unused = {(S.push(std::forward(Parts)), true)...}; +(void)Unused; sbenza wrote: > We could simplify this code if you change `void push(T)` to instead > `StencilPart wrap(T)`. > Then this could be: > > > ``` > Stencil S; > S.Parts = {wrap(std::forward(Parts))...}; > return S; > ``` Much better. I believe the current design came about when StencilPart was move only. Thanks! Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:78 + +template <> bool isEqualData(const RawTextData &A, const RawTextData &B) { + return A.Text == B.Text; sbenza wrote: > Any particular reason to use function template specialization instead of > function overloading? > The former is rare and thus harder to reason about. > > (same for `evalData` below) Stylistic -- it forces you to explicitly declare the "overload set" so to speak. However, i'm not particularly attached -- if you think this a bad idea given its rarity, I'm happy to use overload sets. WDYT? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.
ymandel updated this revision to Diff 195602. ymandel marked 9 inline comments as done. ymandel added a comment. Responded to comments, including changing call operator to return Expected instead of string. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 Files: clang/include/clang/Tooling/Refactoring/Stencil.h clang/lib/Tooling/Refactoring/CMakeLists.txt clang/lib/Tooling/Refactoring/Stencil.cpp clang/unittests/Tooling/CMakeLists.txt clang/unittests/Tooling/StencilTest.cpp Index: clang/unittests/Tooling/StencilTest.cpp === --- /dev/null +++ clang/unittests/Tooling/StencilTest.cpp @@ -0,0 +1,223 @@ +//===- unittest/Tooling/StencilTest.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "clang/Tooling/Refactoring/Stencil.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Tooling/FixIt.h" +#include "clang/Tooling/Tooling.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using namespace clang; +using namespace tooling; +using namespace ast_matchers; + +namespace { +using ::testing::AllOf; +using ::testing::Eq; +using ::testing::HasSubstr; +using MatchResult = MatchFinder::MatchResult; +using tooling::stencil::node; +using tooling::stencil::sNode; +using tooling::stencil::text; + +// In tests, we can't directly match on llvm::Expected since its accessors +// mutate the object. So, we collapse it to an Optional. +static llvm::Optional toOptional(llvm::Expected V) { + if (V) +return *V; + ADD_FAILURE() << "Losing error in conversion to IsSomething: " +<< llvm::toString(V.takeError()); + return llvm::None; +} + +// A very simple matcher for llvm::Optional values. +MATCHER_P(IsSomething, ValueMatcher, "") { + if (!arg) +return false; + return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener); +} + +// Create a valid translation-unit from a statement. +static std::string wrapSnippet(llvm::Twine StatementCode) { + return ("auto stencil_test_snippet = []{" + StatementCode + "};").str(); +} + +static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) { + return varDecl(hasName("stencil_test_snippet"), + hasDescendant(compoundStmt(hasAnySubstatement(Matcher; +} + +struct TestMatch { + // The AST unit from which `result` is built. We bundle it because it backs + // the result. Users are not expected to access it. + std::unique_ptr AstUnit; + // The result to use in the test. References `ast_unit`. + MatchResult Result; +}; + +// Matches `Matcher` against the statement `StatementCode` and returns the +// result. Handles putting the statement inside a function and modifying the +// matcher correspondingly. `Matcher` should match `StatementCode` exactly -- +// that is, produce exactly one match. +static llvm::Optional matchStmt(llvm::Twine StatementCode, + StatementMatcher Matcher) { + auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode)); + if (AstUnit == nullptr) { +ADD_FAILURE() << "AST construction failed"; +return llvm::None; + } + ASTContext &Context = AstUnit->getASTContext(); + auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context); + // We expect a single, exact match for the statement. + if (Matches.size() != 1) { +ADD_FAILURE() << "Wrong number of matches: " << Matches.size(); +return llvm::None; + } + return TestMatch{std::move(AstUnit), MatchResult(Matches[0], &Context)}; +} + +class StencilTest : public ::testing::Test { +protected: + // Verifies that the given stencil fails when evaluated on a valid match + // result. Binds a statement to "stmt", a (non-member) ctor-initializer to + // "init", an expression to "expr" and a (nameless) declaration to "decl". + void testError(const Stencil &Stencil, + ::testing::Matcher Matcher) { +const std::string Snippet = R"cc( + struct A {}; + class F : public A { + public: +F(int) {} + }; + F(1); +)cc"; +auto StmtMatch = matchStmt( +Snippet, +stmt(hasDescendant( + cxxConstructExpr( + hasDeclaration(decl(hasDescendant(cxxCtorInitializer( + isBaseInitializer()) + .bind("init"))) +.bind("decl"))) + .bind("expr"))) +.bind("stmt")); +ASSERT_TRUE(StmtMatch); +if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) { + A
[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.
sbenza added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:45 + /// Evaluates this part to a string and appends it to `result`. + virtual llvm::Error eval(const ast_matchers::MatchFinder::MatchResult &Match, + std::string *Result) const = 0; ymandel wrote: > sbenza wrote: > > Why not use `llvm::Expected` as the return type? > so that each stencil part can append to a single string that we construct > when evaluating the whole stencil. this was an (attempted?) optimization. if > concatenating is about as efficient, I'd prefer that approach. What do you > advise? I think its fine as it is then. I don't really think the performance would matter that much, but I don't see a reason to add all those copies unnecessarily. Mention that `Result` is in an unspecified state in case of error. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:127 + // Allow Stencils to operate as std::function, for compatibility with + // Transformer's TextGenerator. Calls `eval()` and asserts on failure. + std::string operator()(const ast_matchers::MatchFinder::MatchResult &) const; ymandel wrote: > sbenza wrote: > > "asserts" as it only fails in debug mode? > I thought `assert()` also fails in normal mode, based on my reading of the > llvm docs, but it's not explicit about this: > http://llvm.org/docs/ProgrammersManual.html#programmatic-errors > > FWIW, I'm on the fence whether `eval()` should actually have the same > signature and similarly just crash the program on errors. Its not clear > there's any value to propogating the errors up the stack via `Expected`. `assert` follows `NDEBUG`, which is not explicitly related to `-Ox`, but they usually go together. Don't make it crash, please. Otherwise it would be hard to have a refactoring service or alike. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:101 +S.Parts.reserve(sizeof...(Parts)); +auto Unused = {(S.push(std::forward(Parts)), true)...}; +(void)Unused; We could simplify this code if you change `void push(T)` to instead `StencilPart wrap(T)`. Then this could be: ``` Stencil S; S.Parts = {wrap(std::forward(Parts))...}; return S; ``` Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:78 + +template <> bool isEqualData(const RawTextData &A, const RawTextData &B) { + return A.Text == B.Text; Any particular reason to use function template specialization instead of function overloading? The former is rare and thus harder to reason about. (same for `evalData` below) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.
ymandel updated this revision to Diff 194551. ymandel added a comment. tweaks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 Files: clang/include/clang/Tooling/Refactoring/Stencil.h clang/lib/Tooling/Refactoring/CMakeLists.txt clang/lib/Tooling/Refactoring/Stencil.cpp clang/unittests/Tooling/CMakeLists.txt clang/unittests/Tooling/StencilTest.cpp Index: clang/unittests/Tooling/StencilTest.cpp === --- /dev/null +++ clang/unittests/Tooling/StencilTest.cpp @@ -0,0 +1,222 @@ +//===- unittest/Tooling/StencilTest.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "clang/Tooling/Refactoring/Stencil.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Tooling/FixIt.h" +#include "clang/Tooling/Tooling.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using namespace clang; +using namespace tooling; +using namespace ast_matchers; + +namespace { +using ::testing::AllOf; +using ::testing::Eq; +using ::testing::HasSubstr; +using MatchResult = MatchFinder::MatchResult; +using tooling::stencil::node; +using tooling::stencil::sNode; +using tooling::stencil::text; + +// In tests, we can't directly match on llvm::Expected since its accessors +// mutate the object. So, we collapse it to an Optional. +static llvm::Optional toOptional(llvm::Expected V) { + if (V) +return *V; + ADD_FAILURE() << "Losing error in conversion to IsSomething: " +<< llvm::toString(V.takeError()); + return llvm::None; +} + +// A very simple matcher for llvm::Optional values. +MATCHER_P(IsSomething, ValueMatcher, "") { + if (!arg) +return false; + return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener); +} + +// Create a valid translation-unit from a statement. +static std::string wrapSnippet(llvm::Twine StatementCode) { + return ("auto stencil_test_snippet = []{" + StatementCode + "};").str(); +} + +static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) { + return varDecl(hasName("stencil_test_snippet"), + hasDescendant(compoundStmt(hasAnySubstatement(Matcher; +} + +struct TestMatch { + // The AST unit from which `result` is built. We bundle it because it backs + // the result. Users are not expected to access it. + std::unique_ptr AstUnit; + // The result to use in the test. References `ast_unit`. + MatchResult Result; +}; + +// Matches `Matcher` against the statement `StatementCode` and returns the +// result. Handles putting the statement inside a function and modifying the +// matcher correspondingly. `Matcher` should match `StatementCode` exactly -- +// that is, produce exactly one match. +static llvm::Optional matchStmt(llvm::Twine StatementCode, + StatementMatcher Matcher) { + auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode)); + if (AstUnit == nullptr) { +ADD_FAILURE() << "AST construction failed"; +return llvm::None; + } + ASTContext &Context = AstUnit->getASTContext(); + auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context); + // We expect a single, exact match for the statement. + if (Matches.size() != 1) { +ADD_FAILURE() << "Wrong number of matches: " << Matches.size(); +return llvm::None; + } + return TestMatch{std::move(AstUnit), MatchResult(Matches[0], &Context)}; +} + +class StencilTest : public ::testing::Test { +protected: + // Verifies that the given stencil fails when evaluated on a valid match + // result. Binds a statement to "stmt", a (non-member) ctor-initializer to + // "init", an expression to "expr" and a (nameless) declaration to "decl". + void testError(const Stencil &Stencil, + ::testing::Matcher Matcher) { +const std::string Snippet = R"cc( + struct A {}; + class F : public A { + public: +F(int) {} + }; + F(1); +)cc"; +auto StmtMatch = matchStmt( +Snippet, +stmt(hasDescendant( + cxxConstructExpr( + hasDeclaration(decl(hasDescendant(cxxCtorInitializer( + isBaseInitializer()) + .bind("init"))) +.bind("decl"))) + .bind("expr"))) +.bind("stmt")); +ASSERT_TRUE(StmtMatch); +if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) { + ADD_FAILURE() << "Expected failure but succeeded: " << *ResultOrErr; +} else { + auto Err = llvm::handleErrors(ResultOrEr
[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.
ymandel added a comment. Thanks for the review. I've changed most of the things with only a few open questions. PTAL. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:45 + /// Evaluates this part to a string and appends it to `result`. + virtual llvm::Error eval(const ast_matchers::MatchFinder::MatchResult &Match, + std::string *Result) const = 0; sbenza wrote: > Why not use `llvm::Expected` as the return type? so that each stencil part can append to a single string that we construct when evaluating the whole stencil. this was an (attempted?) optimization. if concatenating is about as efficient, I'd prefer that approach. What do you advise? Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:75 + // Copy constructor/assignment produce a deep copy. + StencilPart(const StencilPart &P) : Impl(P.Impl->clone()) {} + StencilPart(StencilPart &&) = default; sbenza wrote: > The interface API is all const. Why not keep a `shared_ptr` instead? > That way we don't have to have users implement a clone function. Excellent! I guess I'm allergic to shared_ptr because of its atomics but its clearly a win here. Thanks! Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:127 + // Allow Stencils to operate as std::function, for compatibility with + // Transformer's TextGenerator. Calls `eval()` and asserts on failure. + std::string operator()(const ast_matchers::MatchFinder::MatchResult &) const; sbenza wrote: > "asserts" as it only fails in debug mode? I thought `assert()` also fails in normal mode, based on my reading of the llvm docs, but it's not explicit about this: http://llvm.org/docs/ProgrammersManual.html#programmatic-errors FWIW, I'm on the fence whether `eval()` should actually have the same signature and similarly just crash the program on errors. Its not clear there's any value to propogating the errors up the stack via `Expected`. Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:45 + +namespace { +// An arbitrary fragment of code within a stencil. sbenza wrote: > maybe this anon namespace should cover the functions above? the style guide advises against. I've been told to only put type definitions inside anon namespaces. Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:76 + +static bool operator==(const RawTextData &A, const RawTextData &B) { + return A.Text == B.Text; sbenza wrote: > If you define ==, imo you should define != too. > Otherwise just call it `isEqual` Since I don't have a general need for these as operators, just went w/ the latter. Used the same style as `evalData()`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.
ymandel updated this revision to Diff 194550. ymandel added a comment. deleted some stray comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 Files: clang/include/clang/Tooling/Refactoring/Stencil.h clang/lib/Tooling/Refactoring/CMakeLists.txt clang/lib/Tooling/Refactoring/Stencil.cpp clang/unittests/Tooling/CMakeLists.txt clang/unittests/Tooling/StencilTest.cpp Index: clang/unittests/Tooling/StencilTest.cpp === --- /dev/null +++ clang/unittests/Tooling/StencilTest.cpp @@ -0,0 +1,222 @@ +//===- unittest/Tooling/StencilTest.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "clang/Tooling/Refactoring/Stencil.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Tooling/FixIt.h" +#include "clang/Tooling/Tooling.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using namespace clang; +using namespace tooling; +using namespace ast_matchers; + +namespace { +using ::testing::AllOf; +using ::testing::Eq; +using ::testing::HasSubstr; +using MatchResult = MatchFinder::MatchResult; +using tooling::stencil::node; +using tooling::stencil::sNode; +using tooling::stencil::text; + +// In tests, we can't directly match on llvm::Expected since its accessors +// mutate the object. So, we collapse it to an Optional. +static llvm::Optional toOptional(llvm::Expected V) { + if (V) +return *V; + ADD_FAILURE() << "Losing error in conversion to IsSomething: " +<< llvm::toString(V.takeError()); + return llvm::None; +} + +// A very simple matcher for llvm::Optional values. +MATCHER_P(IsSomething, ValueMatcher, "") { + if (!arg) +return false; + return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener); +} + +// Create a valid translation-unit from a statement. +static std::string wrapSnippet(llvm::Twine StatementCode) { + return ("auto stencil_test_snippet = []{" + StatementCode + "};").str(); +} + +static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) { + return varDecl(hasName("stencil_test_snippet"), + hasDescendant(compoundStmt(hasAnySubstatement(Matcher; +} + +struct TestMatch { + // The AST unit from which `result` is built. We bundle it because it backs + // the result. Users are not expected to access it. + std::unique_ptr AstUnit; + // The result to use in the test. References `ast_unit`. + MatchResult Result; +}; + +// Matches `Matcher` against the statement `StatementCode` and returns the +// result. Handles putting the statement inside a function and modifying the +// matcher correspondingly. `Matcher` should match `StatementCode` exactly -- +// that is, produce exactly one match. +static llvm::Optional matchStmt(llvm::Twine StatementCode, + StatementMatcher Matcher) { + auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode)); + if (AstUnit == nullptr) { +ADD_FAILURE() << "AST construction failed"; +return llvm::None; + } + ASTContext &Context = AstUnit->getASTContext(); + auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context); + // We expect a single, exact match for the statement. + if (Matches.size() != 1) { +ADD_FAILURE() << "Wrong number of matches: " << Matches.size(); +return llvm::None; + } + return TestMatch{std::move(AstUnit), MatchResult(Matches[0], &Context)}; +} + +class StencilTest : public ::testing::Test { +protected: + // Verifies that the given stencil fails when evaluated on a valid match + // result. Binds a statement to "stmt", a (non-member) ctor-initializer to + // "init", an expression to "expr" and a (nameless) declaration to "decl". + void testError(const Stencil &Stencil, + ::testing::Matcher Matcher) { +const std::string Snippet = R"cc( + struct A {}; + class F : public A { + public: +F(int) {} + }; + F(1); +)cc"; +auto StmtMatch = matchStmt( +Snippet, +stmt(hasDescendant( + cxxConstructExpr( + hasDeclaration(decl(hasDescendant(cxxCtorInitializer( + isBaseInitializer()) + .bind("init"))) +.bind("decl"))) + .bind("expr"))) +.bind("stmt")); +ASSERT_TRUE(StmtMatch); +if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) { + ADD_FAILURE() << "Expected failure but succeeded: " << *ResultOrErr; +} else { + auto Err = llvm::ha
[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.
ymandel updated this revision to Diff 194549. ymandel marked 13 inline comments as done. ymandel added a comment. Responded to comments and added wrapper for Stencil::cat in stencil namespace. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 Files: clang/include/clang/Tooling/Refactoring/Stencil.h clang/lib/Tooling/Refactoring/CMakeLists.txt clang/lib/Tooling/Refactoring/Stencil.cpp clang/unittests/Tooling/CMakeLists.txt clang/unittests/Tooling/StencilTest.cpp Index: clang/unittests/Tooling/StencilTest.cpp === --- /dev/null +++ clang/unittests/Tooling/StencilTest.cpp @@ -0,0 +1,222 @@ +//===- unittest/Tooling/StencilTest.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "clang/Tooling/Refactoring/Stencil.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Tooling/FixIt.h" +#include "clang/Tooling/Tooling.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using namespace clang; +using namespace tooling; +using namespace ast_matchers; + +namespace { +using ::testing::AllOf; +using ::testing::Eq; +using ::testing::HasSubstr; +using MatchResult = MatchFinder::MatchResult; +using tooling::stencil::node; +using tooling::stencil::sNode; +using tooling::stencil::text; + +// In tests, we can't directly match on llvm::Expected since its accessors +// mutate the object. So, we collapse it to an Optional. +static llvm::Optional toOptional(llvm::Expected V) { + if (V) +return *V; + ADD_FAILURE() << "Losing error in conversion to IsSomething: " +<< llvm::toString(V.takeError()); + return llvm::None; +} + +// A very simple matcher for llvm::Optional values. +MATCHER_P(IsSomething, ValueMatcher, "") { + if (!arg) +return false; + return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener); +} + +// Create a valid translation-unit from a statement. +static std::string wrapSnippet(llvm::Twine StatementCode) { + return ("auto stencil_test_snippet = []{" + StatementCode + "};").str(); +} + +static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) { + return varDecl(hasName("stencil_test_snippet"), + hasDescendant(compoundStmt(hasAnySubstatement(Matcher; +} + +struct TestMatch { + // The AST unit from which `result` is built. We bundle it because it backs + // the result. Users are not expected to access it. + std::unique_ptr AstUnit; + // The result to use in the test. References `ast_unit`. + MatchResult Result; +}; + +// Matches `Matcher` against the statement `StatementCode` and returns the +// result. Handles putting the statement inside a function and modifying the +// matcher correspondingly. `Matcher` should match `StatementCode` exactly -- +// that is, produce exactly one match. +static llvm::Optional matchStmt(llvm::Twine StatementCode, + StatementMatcher Matcher) { + auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode)); + if (AstUnit == nullptr) { +ADD_FAILURE() << "AST construction failed"; +return llvm::None; + } + ASTContext &Context = AstUnit->getASTContext(); + auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context); + // We expect a single, exact match for the statement. + if (Matches.size() != 1) { +ADD_FAILURE() << "Wrong number of matches: " << Matches.size(); +return llvm::None; + } + return TestMatch{std::move(AstUnit), MatchResult(Matches[0], &Context)}; +} + +class StencilTest : public ::testing::Test { +protected: + // Verifies that the given stencil fails when evaluated on a valid match + // result. Binds a statement to "stmt", a (non-member) ctor-initializer to + // "init", an expression to "expr" and a (nameless) declaration to "decl". + void testError(const Stencil &Stencil, + ::testing::Matcher Matcher) { +const std::string Snippet = R"cc( + struct A {}; + class F : public A { + public: +F(int) {} + }; + F(1); +)cc"; +auto StmtMatch = matchStmt( +Snippet, +stmt(hasDescendant( + cxxConstructExpr( + hasDeclaration(decl(hasDescendant(cxxCtorInitializer( + isBaseInitializer()) + .bind("init"))) +.bind("decl"))) + .bind("expr"))) +.bind("stmt")); +ASSERT_TRUE(StmtMatch); +if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) { + ADD_FAILURE() <<
[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.
sbenza added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:45 + /// Evaluates this part to a string and appends it to `result`. + virtual llvm::Error eval(const ast_matchers::MatchFinder::MatchResult &Match, + std::string *Result) const = 0; Why not use `llvm::Expected` as the return type? Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:75 + // Copy constructor/assignment produce a deep copy. + StencilPart(const StencilPart &P) : Impl(P.Impl->clone()) {} + StencilPart(StencilPart &&) = default; The interface API is all const. Why not keep a `shared_ptr` instead? That way we don't have to have users implement a clone function. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:94 + return false; +return Impl->isEqual(*(Other.Impl)); + } Parens around Other.Impl are not necessary. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:110 + template static Stencil cat(Ts &&... Parts) { +Stencil Stencil; +Stencil.Parts.reserve(sizeof...(Parts)); I don't like naming the variable the same as the type. You could just use `S` as the variable name. That is ok with llvm style for small functions. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:127 + // Allow Stencils to operate as std::function, for compatibility with + // Transformer's TextGenerator. Calls `eval()` and asserts on failure. + std::string operator()(const ast_matchers::MatchFinder::MatchResult &) const; "asserts" as it only fails in debug mode? Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:130 + + bool operator==(const Stencil &Other) const { +return Parts == Other.Parts; I usually prefer free functions for binary operators (friends is fine). It makes them symmetric. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:155 +/// statement. +StencilPart snode(llvm::StringRef Id); + `sNode` ? ie camelCase Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:45 + +namespace { +// An arbitrary fragment of code within a stencil. maybe this anon namespace should cover the functions above? Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:76 + +static bool operator==(const RawTextData &A, const RawTextData &B) { + return A.Text == B.Text; If you define ==, imo you should define != too. Otherwise just call it `isEqual` Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:94 +Error evalData(const T &Data, const MatchFinder::MatchResult &Match, + std::string *Result); + alignment. (below too) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.
ymandel updated this revision to Diff 194157. ymandel added a comment. Add `operator()` to Stencil for compatability with TextGenerator. Stencil is technically independent of Transformer. However, Transformers replacements and explanations take a generic std::function (there abbreviated as `TextGenerator`). This addition let's Stencils act as TextGenerators. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 Files: clang/include/clang/Tooling/Refactoring/Stencil.h clang/lib/Tooling/Refactoring/CMakeLists.txt clang/lib/Tooling/Refactoring/Stencil.cpp clang/unittests/Tooling/CMakeLists.txt clang/unittests/Tooling/StencilTest.cpp Index: clang/unittests/Tooling/StencilTest.cpp === --- /dev/null +++ clang/unittests/Tooling/StencilTest.cpp @@ -0,0 +1,222 @@ +//===- unittest/Tooling/StencilTest.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "clang/Tooling/Refactoring/Stencil.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Tooling/FixIt.h" +#include "clang/Tooling/Tooling.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using namespace clang; +using namespace tooling; +using namespace ast_matchers; + +namespace { +using ::testing::AllOf; +using ::testing::Eq; +using ::testing::HasSubstr; +using MatchResult = MatchFinder::MatchResult; +using tooling::stencil::node; +using tooling::stencil::snode; +using tooling::stencil::text; + +// In tests, we can't directly match on llvm::Expected since its accessors +// mutate the object. So, we collapse it to an Optional. +static llvm::Optional toOptional(llvm::Expected V) { + if (V) +return *V; + ADD_FAILURE() << "Losing error in conversion to IsSomething: " +<< llvm::toString(V.takeError()); + return llvm::None; +} + +// A very simple matcher for llvm::Optional values. +MATCHER_P(IsSomething, ValueMatcher, "") { + if (!arg) +return false; + return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener); +} + +// Create a valid translation-unit from a statement. +static std::string wrapSnippet(llvm::Twine StatementCode) { + return ("auto stencil_test_snippet = []{" + StatementCode + "};").str(); +} + +static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) { + return varDecl(hasName("stencil_test_snippet"), + hasDescendant(compoundStmt(hasAnySubstatement(Matcher; +} + +struct TestMatch { + // The AST unit from which `result` is built. We bundle it because it backs + // the result. Users are not expected to access it. + std::unique_ptr AstUnit; + // The result to use in the test. References `ast_unit`. + MatchResult Result; +}; + +// Matches `Matcher` against the statement `StatementCode` and returns the +// result. Handles putting the statement inside a function and modifying the +// matcher correspondingly. `Matcher` should match `StatementCode` exactly -- +// that is, produce exactly one match. +static llvm::Optional matchStmt(llvm::Twine StatementCode, + StatementMatcher Matcher) { + auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode)); + if (AstUnit == nullptr) { +ADD_FAILURE() << "AST construction failed"; +return llvm::None; + } + ASTContext &Context = AstUnit->getASTContext(); + auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context); + // We expect a single, exact match for the statement. + if (Matches.size() != 1) { +ADD_FAILURE() << "Wrong number of matches: " << Matches.size(); +return llvm::None; + } + return TestMatch{std::move(AstUnit), MatchResult(Matches[0], &Context)}; +} + +class StencilTest : public ::testing::Test { +protected: + // Verifies that the given stencil fails when evaluated on a valid match + // result. Binds a statement to "stmt", a (non-member) ctor-initializer to + // "init", an expression to "expr" and a (nameless) declaration to "decl". + void testError(const Stencil &Stencil, + ::testing::Matcher Matcher) { +const std::string Snippet = R"cc( + struct A {}; + class F : public A { + public: +F(int) {} + }; + F(1); +)cc"; +auto StmtMatch = matchStmt( +Snippet, +stmt(hasDescendant( + cxxConstructExpr( + hasDeclaration(decl(hasDescendant(cxxCtorInitializer( + isBaseInitializer()) + .bind("init"))) +.bind("decl"))) +
[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.
ymandel updated this revision to Diff 193906. ymandel added a comment. Adjust to use SourceCode instead of FixIt. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 Files: clang/include/clang/Tooling/Refactoring/Stencil.h clang/lib/Tooling/Refactoring/CMakeLists.txt clang/lib/Tooling/Refactoring/Stencil.cpp clang/unittests/Tooling/CMakeLists.txt clang/unittests/Tooling/StencilTest.cpp Index: clang/unittests/Tooling/StencilTest.cpp === --- /dev/null +++ clang/unittests/Tooling/StencilTest.cpp @@ -0,0 +1,204 @@ +//===- unittest/Tooling/StencilTest.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "clang/Tooling/Refactoring/Stencil.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Tooling/FixIt.h" +#include "clang/Tooling/Tooling.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using namespace clang; +using namespace tooling; +using namespace ast_matchers; + +namespace { +using ::testing::AllOf; +using ::testing::Eq; +using ::testing::HasSubstr; +using MatchResult = MatchFinder::MatchResult; +using tooling::stencil::node; +using tooling::stencil::snode; +using tooling::stencil::text; + +// In tests, we can't directly match on llvm::Expected since its accessors +// mutate the object. So, we collapse it to an Optional. +static llvm::Optional toOptional(llvm::Expected V) { + if (V) +return *V; + ADD_FAILURE() << "Losing error in conversion to IsSomething: " +<< llvm::toString(V.takeError()); + return llvm::None; +} + +// A very simple matcher for llvm::Optional values. +MATCHER_P(IsSomething, ValueMatcher, "") { + if (!arg) +return false; + return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener); +} + +// Create a valid translation-unit from a statement. +static std::string wrapSnippet(llvm::Twine StatementCode) { + return ("auto stencil_test_snippet = []{" + StatementCode + "};").str(); +} + +static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) { + return varDecl(hasName("stencil_test_snippet"), + hasDescendant(compoundStmt(hasAnySubstatement(Matcher; +} + +struct TestMatch { + // The AST unit from which `result` is built. We bundle it because it backs + // the result. Users are not expected to access it. + std::unique_ptr AstUnit; + // The result to use in the test. References `ast_unit`. + MatchResult Result; +}; + +// Matches `Matcher` against the statement `StatementCode` and returns the +// result. Handles putting the statement inside a function and modifying the +// matcher correspondingly. `Matcher` should match `StatementCode` exactly -- +// that is, produce exactly one match. +static llvm::Optional matchStmt(llvm::Twine StatementCode, + StatementMatcher Matcher) { + auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode)); + if (AstUnit == nullptr) { +ADD_FAILURE() << "AST construction failed"; +return llvm::None; + } + ASTContext &Context = AstUnit->getASTContext(); + auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context); + // We expect a single, exact match for the statement. + if (Matches.size() != 1) { +ADD_FAILURE() << "Wrong number of matches: " << Matches.size(); +return llvm::None; + } + return TestMatch{std::move(AstUnit), MatchResult(Matches[0], &Context)}; +} + +class StencilTest : public ::testing::Test { +protected: + // Verifies that the given stencil fails when evaluated on a valid match + // result. Binds a statement to "stmt", a (non-member) ctor-initializer to + // "init", an expression to "expr" and a (nameless) declaration to "decl". + void testError(const Stencil &Stencil, + ::testing::Matcher Matcher) { +const std::string Snippet = R"cc( + struct A {}; + class F : public A { + public: +F(int) {} + }; + F(1); +)cc"; +auto StmtMatch = matchStmt( +Snippet, +stmt(hasDescendant( + cxxConstructExpr( + hasDeclaration(decl(hasDescendant(cxxCtorInitializer( + isBaseInitializer()) + .bind("init"))) +.bind("decl"))) + .bind("expr"))) +.bind("stmt")); +ASSERT_TRUE(StmtMatch); +if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) { + ADD_FAILURE() << "Expected failure but succeeded: " << *ResultOrErr; +} else { + auto
[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.
ymandel updated this revision to Diff 193498. ymandel added a comment. Remove noisy default-defined constructors/operators Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 Files: clang/include/clang/Tooling/Refactoring/Stencil.h clang/lib/Tooling/Refactoring/CMakeLists.txt clang/lib/Tooling/Refactoring/Stencil.cpp clang/unittests/Tooling/CMakeLists.txt clang/unittests/Tooling/StencilTest.cpp Index: clang/unittests/Tooling/StencilTest.cpp === --- /dev/null +++ clang/unittests/Tooling/StencilTest.cpp @@ -0,0 +1,204 @@ +//===- unittest/Tooling/StencilTest.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "clang/Tooling/Refactoring/Stencil.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Tooling/FixIt.h" +#include "clang/Tooling/Tooling.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using namespace clang; +using namespace tooling; +using namespace ast_matchers; + +namespace { +using ::testing::AllOf; +using ::testing::Eq; +using ::testing::HasSubstr; +using MatchResult = MatchFinder::MatchResult; +using tooling::stencil_generators::node; +using tooling::stencil_generators::snode; +using tooling::stencil_generators::text; + +// In tests, we can't directly match on llvm::Expected since its accessors +// mutate the object. So, we collapse it to an Optional. +static llvm::Optional toOptional(llvm::Expected V) { + if (V) +return *V; + ADD_FAILURE() << "Losing error in conversion to IsSomething: " +<< llvm::toString(V.takeError()); + return llvm::None; +} + +// A very simple matcher for llvm::Optional values. +MATCHER_P(IsSomething, ValueMatcher, "") { + if (!arg) +return false; + return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener); +} + +// Create a valid translation-unit from a statement. +static std::string wrapSnippet(llvm::Twine StatementCode) { + return ("auto stencil_test_snippet = []{" + StatementCode + "};").str(); +} + +static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) { + return varDecl(hasName("stencil_test_snippet"), + hasDescendant(compoundStmt(hasAnySubstatement(Matcher; +} + +struct TestMatch { + // The AST unit from which `result` is built. We bundle it because it backs + // the result. Users are not expected to access it. + std::unique_ptr AstUnit; + // The result to use in the test. References `ast_unit`. + MatchResult Result; +}; + +// Matches `Matcher` against the statement `StatementCode` and returns the +// result. Handles putting the statement inside a function and modifying the +// matcher correspondingly. `Matcher` should match `StatementCode` exactly -- +// that is, produce exactly one match. +static llvm::Optional matchStmt(llvm::Twine StatementCode, + StatementMatcher Matcher) { + auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode)); + if (AstUnit == nullptr) { +ADD_FAILURE() << "AST construction failed"; +return llvm::None; + } + ASTContext &Context = AstUnit->getASTContext(); + auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context); + // We expect a single, exact match for the statement. + if (Matches.size() != 1) { +ADD_FAILURE() << "Wrong number of matches: " << Matches.size(); +return llvm::None; + } + return TestMatch{std::move(AstUnit), MatchResult(Matches[0], &Context)}; +} + +class StencilTest : public ::testing::Test { +protected: + // Verifies that the given stencil fails when evaluated on a valid match + // result. Binds a statement to "stmt", a (non-member) ctor-initializer to + // "init", an expression to "expr" and a (nameless) declaration to "decl". + void testError(const Stencil &Stencil, + ::testing::Matcher Matcher) { +const std::string Snippet = R"cc( + struct A {}; + class F : public A { + public: +F(int) {} + }; + F(1); +)cc"; +auto StmtMatch = matchStmt( +Snippet, +stmt(hasDescendant( + cxxConstructExpr( + hasDeclaration(decl(hasDescendant(cxxCtorInitializer( + isBaseInitializer()) + .bind("init"))) +.bind("decl"))) + .bind("expr"))) +.bind("stmt")); +ASSERT_TRUE(StmtMatch); +if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) { + ADD_FAILURE() << "Expected failure but succeeded: " <
[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.
ymandel updated this revision to Diff 193496. ymandel added a comment. Sever dependency on NodeId and some general cleanup. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 Files: clang/include/clang/Tooling/Refactoring/Stencil.h clang/lib/Tooling/Refactoring/CMakeLists.txt clang/lib/Tooling/Refactoring/Stencil.cpp clang/unittests/Tooling/CMakeLists.txt clang/unittests/Tooling/StencilTest.cpp Index: clang/unittests/Tooling/StencilTest.cpp === --- /dev/null +++ clang/unittests/Tooling/StencilTest.cpp @@ -0,0 +1,204 @@ +//===- unittest/Tooling/StencilTest.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "clang/Tooling/Refactoring/Stencil.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Tooling/FixIt.h" +#include "clang/Tooling/Tooling.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using namespace clang; +using namespace tooling; +using namespace ast_matchers; + +namespace { +using ::testing::AllOf; +using ::testing::Eq; +using ::testing::HasSubstr; +using MatchResult = MatchFinder::MatchResult; +using tooling::stencil_generators::node; +using tooling::stencil_generators::snode; +using tooling::stencil_generators::text; + +// In tests, we can't directly match on llvm::Expected since its accessors +// mutate the object. So, we collapse it to an Optional. +static llvm::Optional toOptional(llvm::Expected V) { + if (V) +return *V; + ADD_FAILURE() << "Losing error in conversion to IsSomething: " +<< llvm::toString(V.takeError()); + return llvm::None; +} + +// A very simple matcher for llvm::Optional values. +MATCHER_P(IsSomething, ValueMatcher, "") { + if (!arg) +return false; + return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener); +} + +// Create a valid translation-unit from a statement. +static std::string wrapSnippet(llvm::Twine StatementCode) { + return ("auto stencil_test_snippet = []{" + StatementCode + "};").str(); +} + +static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) { + return varDecl(hasName("stencil_test_snippet"), + hasDescendant(compoundStmt(hasAnySubstatement(Matcher; +} + +struct TestMatch { + // The AST unit from which `result` is built. We bundle it because it backs + // the result. Users are not expected to access it. + std::unique_ptr AstUnit; + // The result to use in the test. References `ast_unit`. + MatchResult Result; +}; + +// Matches `Matcher` against the statement `StatementCode` and returns the +// result. Handles putting the statement inside a function and modifying the +// matcher correspondingly. `Matcher` should match `StatementCode` exactly -- +// that is, produce exactly one match. +static llvm::Optional matchStmt(llvm::Twine StatementCode, + StatementMatcher Matcher) { + auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode)); + if (AstUnit == nullptr) { +ADD_FAILURE() << "AST construction failed"; +return llvm::None; + } + ASTContext &Context = AstUnit->getASTContext(); + auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context); + // We expect a single, exact match for the statement. + if (Matches.size() != 1) { +ADD_FAILURE() << "Wrong number of matches: " << Matches.size(); +return llvm::None; + } + return TestMatch{std::move(AstUnit), MatchResult(Matches[0], &Context)}; +} + +class StencilTest : public ::testing::Test { +protected: + // Verifies that the given stencil fails when evaluated on a valid match + // result. Binds a statement to "stmt", a (non-member) ctor-initializer to + // "init", an expression to "expr" and a (nameless) declaration to "decl". + void testError(const Stencil &Stencil, + ::testing::Matcher Matcher) { +const std::string Snippet = R"cc( + struct A {}; + class F : public A { + public: +F(int) {} + }; + F(1); +)cc"; +auto StmtMatch = matchStmt( +Snippet, +stmt(hasDescendant( + cxxConstructExpr( + hasDeclaration(decl(hasDescendant(cxxCtorInitializer( + isBaseInitializer()) + .bind("init"))) +.bind("decl"))) + .bind("expr"))) +.bind("stmt")); +ASSERT_TRUE(StmtMatch); +if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) { + ADD_FAILURE() << "Expected failure but succeeded: "
[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.
ymandel added a comment. gentle ping... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.
ymandel updated this revision to Diff 190683. ymandel added a comment. Build fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 Files: clang/include/clang/Tooling/Refactoring/Stencil.h clang/lib/Tooling/Refactoring/CMakeLists.txt clang/lib/Tooling/Refactoring/Stencil.cpp clang/unittests/Tooling/CMakeLists.txt clang/unittests/Tooling/StencilTest.cpp Index: clang/unittests/Tooling/StencilTest.cpp === --- /dev/null +++ clang/unittests/Tooling/StencilTest.cpp @@ -0,0 +1,250 @@ +//===- unittest/Tooling/StencilTest.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "clang/Tooling/Refactoring/Stencil.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Tooling/FixIt.h" +#include "clang/Tooling/Tooling.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace clang { +namespace tooling { +namespace { + +using ::clang::ast_matchers::compoundStmt; +using ::clang::ast_matchers::decl; +using ::clang::ast_matchers::declStmt; +using ::clang::ast_matchers::expr; +using ::clang::ast_matchers::hasAnySubstatement; +using ::clang::ast_matchers::hasCondition; +using ::clang::ast_matchers::hasDescendant; +using ::clang::ast_matchers::hasElse; +using ::clang::ast_matchers::hasInitializer; +using ::clang::ast_matchers::hasName; +using ::clang::ast_matchers::hasReturnValue; +using ::clang::ast_matchers::hasSingleDecl; +using ::clang::ast_matchers::hasThen; +using ::clang::ast_matchers::ifStmt; +using ::clang::ast_matchers::ignoringImplicit; +using ::clang::ast_matchers::returnStmt; +using ::clang::ast_matchers::stmt; +using ::clang::ast_matchers::varDecl; + +using MatchResult = ::clang::ast_matchers::MatchFinder::MatchResult; + +using ::clang::tooling::stencil_generators::node; +using ::clang::tooling::stencil_generators::text; + +using ::testing::Eq; + +// In tests, we can't directly match on llvm::Expected since its accessors +// mutate the object. So, we collapse it to an Optional. +llvm::Optional toOptional(llvm::Expected V) { + if (V) +return *V; + ADD_FAILURE() << "Losing error in conversion to IsSomething: " +<< llvm::toString(V.takeError()); + return llvm::None; +} + +// A very simple matcher for llvm::Optional values. +MATCHER_P(IsSomething, ValueMatcher, "") { + if (!arg) +return false; + return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener); +} + +// Create a valid translation-unit from a statement. +std::string wrapSnippet(llvm::Twine StatementCode) { + return ("auto stencil_test_snippet = []{" + StatementCode + "};").str(); +} + +clang::ast_matchers::DeclarationMatcher +wrapMatcher(const clang::ast_matchers::StatementMatcher &Matcher) { + return varDecl(hasName("stencil_test_snippet"), + hasDescendant(compoundStmt(hasAnySubstatement(Matcher; +} + +struct TestMatch { + // The AST unit from which `result` is built. We bundle it because it backs + // the result. Users are not expected to access it. + std::unique_ptr AstUnit; + // The result to use in the test. References `ast_unit`. + MatchResult Result; +}; + +// Matches `Matcher` against the statement `StatementCode` and returns the +// result. Handles putting the statement inside a function and modifying the +// matcher correspondingly. `Matcher` should match `StatementCode` exactly -- +// that is, produce exactly one match. +llvm::Optional +matchStmt(llvm::Twine StatementCode, + clang::ast_matchers::StatementMatcher Matcher) { + auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode)); + if (AstUnit == nullptr) { +ADD_FAILURE() << "AST construction failed"; +return llvm::None; + } + clang::ASTContext &Context = AstUnit->getASTContext(); + auto Matches = clang::ast_matchers::match(wrapMatcher(Matcher), Context); + // We expect a single, exact match for the statement. + if (Matches.size() != 1) { +ADD_FAILURE() << "Wrong number of matches: " << Matches.size(); +return llvm::None; + } + return TestMatch{std::move(AstUnit), MatchResult(Matches[0], &Context)}; +} + +class StencilTest : public ::testing::Test { +protected: + // Verifies that the given stencil fails when evaluated on a valid match + // result. Binds a statement to "stmt", a (non-member) ctor-initializer to + // "init", an expression to "expr" and a (nameless) declaration to "decl". + void testError(const Stencil &Stencil, + testing::Matcher Matcher) { +using ::clang::ast_matchers::cxxConstructExpr; +using ::clang::ast_matchers::cxxCtorInitializer; +using ::clang::ast_matchers::has
[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.
ymandel created this revision. ymandel added a reviewer: ilya-biryukov. ymandel added a project: clang. Herald added subscribers: jdoerfert, jfb, mgorny. This file defines the *Stencil* abstraction: a code-generating object, parameterized by named references to (bound) AST nodes. Given a match result, a stencil can be evaluated to a string of source code. A stencil is similar in spirit to a format string: it is composed of a series of raw text strings, references to nodes (the parameters) and helper code-generation operations. See thread on cfe-dev list with subject "[RFC] Easier source-to-source transformations with clang tooling" for background. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D59371 Files: clang/include/clang/Tooling/Refactoring/Stencil.h clang/lib/Tooling/Refactoring/CMakeLists.txt clang/lib/Tooling/Refactoring/Stencil.cpp clang/unittests/Tooling/CMakeLists.txt clang/unittests/Tooling/StencilTest.cpp Index: clang/unittests/Tooling/StencilTest.cpp === --- /dev/null +++ clang/unittests/Tooling/StencilTest.cpp @@ -0,0 +1,250 @@ +//===- unittest/Tooling/StencilTest.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "clang/Tooling/Refactoring/Stencil.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Tooling/FixIt.h" +#include "clang/Tooling/Tooling.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace clang { +namespace tooling { +namespace { + +using ::clang::ast_matchers::compoundStmt; +using ::clang::ast_matchers::decl; +using ::clang::ast_matchers::declStmt; +using ::clang::ast_matchers::expr; +using ::clang::ast_matchers::hasAnySubstatement; +using ::clang::ast_matchers::hasCondition; +using ::clang::ast_matchers::hasDescendant; +using ::clang::ast_matchers::hasElse; +using ::clang::ast_matchers::hasInitializer; +using ::clang::ast_matchers::hasName; +using ::clang::ast_matchers::hasReturnValue; +using ::clang::ast_matchers::hasSingleDecl; +using ::clang::ast_matchers::hasThen; +using ::clang::ast_matchers::ifStmt; +using ::clang::ast_matchers::ignoringImplicit; +using ::clang::ast_matchers::returnStmt; +using ::clang::ast_matchers::stmt; +using ::clang::ast_matchers::varDecl; + +using MatchResult = ::clang::ast_matchers::MatchFinder::MatchResult; + +using ::clang::tooling::stencil_generators::node; +using ::clang::tooling::stencil_generators::text; + +using ::testing::Eq; + +// In tests, we can't directly match on llvm::Expected since its accessors +// mutate the object. So, we collapse it to an Optional. +llvm::Optional toOptional(llvm::Expected V) { + if (V) +return *V; + ADD_FAILURE() << "Losing error in conversion to IsSomething: " +<< llvm::toString(V.takeError()); + return llvm::None; +} + +// A very simple matcher for llvm::Optional values. +MATCHER_P(IsSomething, ValueMatcher, "") { + if (!arg) +return false; + return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener); +} + +// Create a valid translation-unit from a statement. +std::string wrapSnippet(llvm::Twine StatementCode) { + return ("auto stencil_test_snippet = []{" + StatementCode + "};").str(); +} + +clang::ast_matchers::DeclarationMatcher +wrapMatcher(const clang::ast_matchers::StatementMatcher &Matcher) { + return varDecl(hasName("stencil_test_snippet"), + hasDescendant(compoundStmt(hasAnySubstatement(Matcher; +} + +struct TestMatch { + // The AST unit from which `result` is built. We bundle it because it backs + // the result. Users are not expected to access it. + std::unique_ptr AstUnit; + // The result to use in the test. References `ast_unit`. + MatchResult Result; +}; + +// Matches `Matcher` against the statement `StatementCode` and returns the +// result. Handles putting the statement inside a function and modifying the +// matcher correspondingly. `Matcher` should match `StatementCode` exactly -- +// that is, produce exactly one match. +llvm::Optional +matchStmt(llvm::Twine StatementCode, + clang::ast_matchers::StatementMatcher Matcher) { + auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode)); + if (AstUnit == nullptr) { +ADD_FAILURE() << "AST construction failed"; +return llvm::None; + } + clang::ASTContext &Context = AstUnit->getASTContext(); + auto Matches = clang::ast_matchers::match(wrapMatcher(Matcher), Context); + // We expect a single, exact match for the statement. + if (Matches.size() != 1) { +ADD_FAILURE() << "Wrong number of matches: " << Matches.size(); +return llvm::None; + } + return TestMatch{std::move(AstUnit), MatchResult(Matches[0], &Context)}; +} + +class