On Mon, Aug 20, 2012 at 1:22 PM, Sean Silva <[email protected]> wrote: > I'm not following you. > > If a patch doesn't have enough information to uniquely identify the > source file(s) that it refers to, it would be impossible to apply, > defeating the purpose of having a patch in the first place.
Humans are very good at figuring out context - for a machine that's often harder. Obviously not impossible, but the question is one of whether it's worth to spend the effort, and what the opportunity cost is. > > --Sean Silva > > On Mon, Aug 20, 2012 at 3:58 PM, Manuel Klimek <[email protected]> wrote: >> On Mon, Aug 20, 2012 at 12:37 PM, Sean Silva <[email protected]> wrote: >>> Phabricator feedback: >>> >>> Would it be possible for the "table of contents" (or somewhere else >>> obvious on the page, like maybe near the diffs) to contain links to >>> the original source code of the file, and not just the diff? I felt >>> myself wanting to browse the whole ASTMatchers.h but in not finding a >>> link in the end I fired up a terminal to look at it. >> >> Unfortunately that's hard in general, because the patch might contain >> arbitrarily small subsets at the end of the real path which we'd need >> to match up with the repo... >> >> Fortunately we have a workaround: uploading patches with a large >> context setting (so that the whole file is included in the uploaded >> diff) fixes the immediate problem, and phab is smart enough to output >> and attach a small-context patch on the review mail. >> >> Cheers, >> /Manuel >> >>> >>> Thanks, >>> >>> --Sean Silva >>> >>> On Mon, Aug 20, 2012 at 2:36 PM, Sam Panzer >>> <[email protected]> wrote: >>>> Hi klimek, >>>> >>>> A new matcher which corresponds to the MaterializeTemporaryExpression AST >>>> node. >>>> >>>> http://llvm-reviews.chandlerc.com/D20 >>>> >>>> Files: >>>> include/clang/ASTMatchers/ASTMatchers.h >>>> unittests/ASTMatchers/ASTMatchersTest.cpp >>>> >>>> Index: include/clang/ASTMatchers/ASTMatchers.h >>>> =================================================================== >>>> --- include/clang/ASTMatchers/ASTMatchers.h >>>> +++ include/clang/ASTMatchers/ASTMatchers.h >>>> @@ -473,6 +473,22 @@ >>>> Stmt, >>>> CXXBindTemporaryExpr> bindTemporaryExpression; >>>> >>>> +/// \brief Matches nodes where temporaries are materialized. >>>> +/// >>>> +/// Example: Given >>>> +/// struct T {void func()}; >>>> +/// T f(); >>>> +/// void g(T); >>>> +/// materializeTempExpr() matches 'f()' in these statements >>>> +/// T u(f()); >>>> +/// g(f()); >>>> +/// but does not match >>>> +/// f(); >>>> +/// f().func(); >>>> +const internal::VariadicDynCastAllOfMatcher< >>>> + Stmt, >>>> + MaterializeTemporaryExpr> materializeTempExpr; >>>> + >>>> /// \brief Matches new expressions. >>>> /// >>>> /// Given >>>> Index: unittests/ASTMatchers/ASTMatchersTest.cpp >>>> =================================================================== >>>> --- unittests/ASTMatchers/ASTMatchersTest.cpp >>>> +++ unittests/ASTMatchers/ASTMatchersTest.cpp >>>> @@ -1249,6 +1249,43 @@ >>>> TempExpression)); >>>> } >>>> >>>> +TEST(MaterializeTempExpr, MatchesTemporary) { >>>> + StatementMatcher MaterializeTemp = materializeTempExpr(); >>>> + >>>> + std::string ClassString = >>>> + "class string { public: string(); int length(); }; "; >>>> + >>>> + EXPECT_TRUE( >>>> + matches(ClassString + >>>> + "string GetStringByValue();" >>>> + "void FunctionTakesString(string s);" >>>> + "void run() { FunctionTakesString(GetStringByValue()); }", >>>> + MaterializeTemp)); >>>> + >>>> + EXPECT_TRUE( >>>> + notMatches(ClassString + >>>> + "string* GetStringPointer(); " >>>> + "void FunctionTakesStringPtr(string* s);" >>>> + "void run() {" >>>> + " string* s = GetStringPointer();" >>>> + " FunctionTakesStringPtr(GetStringPointer());" >>>> + " FunctionTakesStringPtr(s);" >>>> + "}", >>>> + MaterializeTemp)); >>>> + >>>> + EXPECT_TRUE( >>>> + notMatches(ClassString + >>>> + "string GetStringByValue();" >>>> + "void run() { int k = GetStringByValue().length(); }", >>>> + MaterializeTemp)); >>>> + >>>> + EXPECT_TRUE( >>>> + notMatches(ClassString + >>>> + "string GetStringByValue();" >>>> + "void run() { GetStringByValue(); }", >>>> + MaterializeTemp)); >>>> +} >>>> + >>>> TEST(ConstructorDeclaration, SimpleCase) { >>>> EXPECT_TRUE(matches("class Foo { Foo(int i); };", >>>> constructor(ofClass(hasName("Foo"))))); >>>> >>>> _______________________________________________ >>>> cfe-commits mailing list >>>> [email protected] >>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>> _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
