r189500 - Add support for eachOf/allOf/anyOf variadic matchers in the dynamic layer.

Aaron Ballman aaron at aaronballman.com
Wed Aug 28 13:31:29 PDT 2013


In MSVC 11, I am now getting:

240>  FAIL: Clang-Unit ::
ASTMatchers/Dynamic/Debug/DynamicASTMatchersTests.exe/RegistryTest.VariadicOp
(6209 of 6552)
240>  ******************** TEST 'Clang-Unit ::
ASTMatchers/Dynamic/Debug/DynamicASTMatchersTests.exe/RegistryTest.VariadicOp'
FAILED ********************
240>  Note: Google Test filter = RegistryTest.VariadicOp
240>
240>  [==========] Running 1 test from 1 test case.
240>
240>  [----------] Global test environment set-up.
240>
240>  [----------] 1 test from RegistryTest
240>
240>  [ RUN      ] RegistryTest.VariadicOp
240>
240>F:\official-llvm\llvm\tools\clang\unittests\ASTMatchers\Dynamic\RegistryTest.cpp(273):
error : Value of: matches("int i = 0;", D)
240>
240>    Actual: true
240>
240>  Expected: false
240>
240>  [  FAILED  ] RegistryTest.VariadicOp (110 ms)
240>
240>  [----------] 1 test from RegistryTest (110 ms total)
240>
240>
240>
240>  [----------] Global test environment tear-down
240>
240>  [==========] 1 test from 1 test case ran. (110 ms total)
240>
240>  [  PASSED  ] 0 tests.
240>
240>  [  FAILED  ] 1 test, listed below:
240>
240>  [  FAILED  ] RegistryTest.VariadicOp
240>
240>
240>
240>   1 FAILED TEST
240>
240>
240>  ********************
240>
240>  Testing Time: 138.30s
240>  ********************
240>  Failing Tests (1):
240>      Clang-Unit ::
ASTMatchers/Dynamic/Debug/DynamicASTMatchersTests.exe/RegistryTest.VariadicOp
240>
240>    Expected Passes    : 6491
240>    Expected Failures  : 26
240>    Unsupported Tests  : 34
240>    Unexpected Failures: 1
240>  lit.py: LitConfig.py:98: note: Unable to find 'bash.exe'.
240>  lit.py: <string>:187: note: using clang:
'F:/official-llvm/build/bin/Debug/clang.EXE'

The builders appear to agree:

http://bb.pgr.jp/builders/cmake-clang-x64-msc16-R/builds/2615/steps/test_clang/logs/Clang-Unit%20%3A%3A%20ASTMatchers__Dynamic__Release__DynamicASTMatchersTests.exe__RegistryTest.VariadicOp

Can you fix or revert?  Thanks!

~Aaron

On Wed, Aug 28, 2013 at 2:42 PM, Samuel Benzaquen <sbenza at google.com> wrote:
> Author: sbenza
> Date: Wed Aug 28 13:42:04 2013
> New Revision: 189500
>
> URL: http://llvm.org/viewvc/llvm-project?rev=189500&view=rev
> Log:
> Add support for eachOf/allOf/anyOf variadic matchers in the dynamic layer.
>
> Summary:
> Add support for eachOf/allOf/anyOf variadic matchers in the dynamic layer.
> These function require some late binding behavior for the type conversions, thus changes in VariadicValue's MatcherList.
> Second try. This time with a fix for C++11 builds.
>
> Reviewers: klimek
>
> CC: cfe-commits, revane
>
> Differential Revision: http://llvm-reviews.chandlerc.com/D1536
>
> Modified:
>     cfe/trunk/include/clang/ASTMatchers/Dynamic/VariantValue.h
>     cfe/trunk/lib/ASTMatchers/Dynamic/Marshallers.h
>     cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
>     cfe/trunk/lib/ASTMatchers/Dynamic/VariantValue.cpp
>     cfe/trunk/unittests/ASTMatchers/Dynamic/ParserTest.cpp
>     cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
>
> Modified: cfe/trunk/include/clang/ASTMatchers/Dynamic/VariantValue.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/Dynamic/VariantValue.h?rev=189500&r1=189499&r2=189500&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/ASTMatchers/Dynamic/VariantValue.h (original)
> +++ cfe/trunk/include/clang/ASTMatchers/Dynamic/VariantValue.h Wed Aug 28 13:42:04 2013
> @@ -50,6 +50,10 @@ class VariantMatcher {
>    public:
>      virtual ~MatcherOps();
>      virtual bool canConstructFrom(const DynTypedMatcher &Matcher) const = 0;
> +    virtual void constructFrom(const DynTypedMatcher &Matcher) = 0;
> +    virtual void constructVariadicOperator(
> +        ast_matchers::internal::VariadicOperatorFunction Func,
> +        ArrayRef<VariantMatcher> InnerMatchers) = 0;
>    };
>
>    /// \brief Payload interface to be specialized by each matcher type.
> @@ -60,9 +64,7 @@ class VariantMatcher {
>      virtual ~Payload();
>      virtual bool getSingleMatcher(const DynTypedMatcher *&Out) const = 0;
>      virtual std::string getTypeAsString() const = 0;
> -    virtual bool hasTypedMatcher(const MatcherOps &Ops) const = 0;
> -    virtual const DynTypedMatcher *
> -    getTypedMatcher(const MatcherOps &Ops) const = 0;
> +    virtual void makeTypedMatcher(MatcherOps &Ops) const = 0;
>    };
>
>  public:
> @@ -78,6 +80,13 @@ public:
>    static VariantMatcher
>    PolymorphicMatcher(ArrayRef<const DynTypedMatcher *> Matchers);
>
> +  /// \brief Creates a 'variadic' operator matcher.
> +  ///
> +  /// It will bind to the appropriate type on getTypedMatcher<T>().
> +  static VariantMatcher VariadicOperatorMatcher(
> +      ast_matchers::internal::VariadicOperatorFunction Func,
> +      ArrayRef<VariantMatcher> Args);
> +
>    /// \brief Makes the matcher the "null" matcher.
>    void reset();
>
> @@ -101,8 +110,9 @@ public:
>    /// that can, the result would be ambiguous and false is returned.
>    template <class T>
>    bool hasTypedMatcher() const {
> -    if (Value) return Value->hasTypedMatcher(TypedMatcherOps<T>());
> -    return false;
> +    TypedMatcherOps<T> Ops;
> +    if (Value) Value->makeTypedMatcher(Ops);
> +    return Ops.hasMatcher();
>    }
>
>    /// \brief Return this matcher as a \c Matcher<T>.
> @@ -111,9 +121,10 @@ public:
>    /// Asserts that \c hasTypedMatcher<T>() is true.
>    template <class T>
>    ast_matchers::internal::Matcher<T> getTypedMatcher() const {
> -    assert(hasTypedMatcher<T>());
> -    return ast_matchers::internal::Matcher<T>::constructFrom(
> -        *Value->getTypedMatcher(TypedMatcherOps<T>()));
> +    TypedMatcherOps<T> Ops;
> +    Value->makeTypedMatcher(Ops);
> +    assert(Ops.hasMatcher() && "hasTypedMatcher<T>() == false");
> +    return Ops.matcher();
>    }
>
>    /// \brief String representation of the type of the value.
> @@ -127,13 +138,50 @@ private:
>
>    class SinglePayload;
>    class PolymorphicPayload;
> +  class VariadicOpPayload;
>
>    template <typename T>
>    class TypedMatcherOps : public MatcherOps {
>    public:
> +    typedef ast_matchers::internal::Matcher<T> MatcherT;
> +
>      virtual bool canConstructFrom(const DynTypedMatcher &Matcher) const {
> -      return ast_matchers::internal::Matcher<T>::canConstructFrom(Matcher);
> +      return MatcherT::canConstructFrom(Matcher);
> +    }
> +
> +    virtual void constructFrom(const DynTypedMatcher& Matcher) {
> +      Out.reset(new MatcherT(MatcherT::constructFrom(Matcher)));
>      }
> +
> +    virtual void constructVariadicOperator(
> +        ast_matchers::internal::VariadicOperatorFunction Func,
> +        ArrayRef<VariantMatcher> InnerMatchers) {
> +      const size_t NumArgs = InnerMatchers.size();
> +      MatcherT **InnerArgs = new MatcherT *[NumArgs]();
> +      bool HasError = false;
> +      for (size_t i = 0; i != NumArgs; ++i) {
> +        // Abort if any of the inner matchers can't be converted to
> +        // Matcher<T>.
> +        if (!InnerMatchers[i].hasTypedMatcher<T>()) {
> +          HasError = true;
> +          break;
> +        }
> +        InnerArgs[i] = new MatcherT(InnerMatchers[i].getTypedMatcher<T>());
> +      }
> +      if (!HasError) {
> +        Out.reset(new MatcherT(
> +            new ast_matchers::internal::VariadicOperatorMatcherInterface<T>(
> +                Func, ArrayRef<const MatcherT *>(InnerArgs, NumArgs))));
> +      }
> +      std::for_each(InnerArgs, InnerArgs + NumArgs, llvm::deleter<MatcherT>);
> +      delete[] InnerArgs;
> +    }
> +
> +    bool hasMatcher() const { return Out.get() != NULL; }
> +    const MatcherT &matcher() const { return *Out; }
> +
> +  private:
> +    OwningPtr<MatcherT> Out;
>    };
>
>    IntrusiveRefCntPtr<const Payload> Value;
>
> Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Marshallers.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/Marshallers.h?rev=189500&r1=189499&r2=189500&view=diff
> ==============================================================================
> --- cfe/trunk/lib/ASTMatchers/Dynamic/Marshallers.h (original)
> +++ cfe/trunk/lib/ASTMatchers/Dynamic/Marshallers.h Wed Aug 28 13:42:04 2013
> @@ -348,6 +348,36 @@ private:
>    std::vector<MatcherCreateCallback *> Overloads;
>  };
>
> +/// \brief Variadic operator marshaller function.
> +class VariadicOperatorMatcherCreateCallback : public MatcherCreateCallback {
> +public:
> +  typedef ast_matchers::internal::VariadicOperatorFunction VarFunc;
> +  VariadicOperatorMatcherCreateCallback(VarFunc Func, StringRef MatcherName)
> +      : Func(Func), MatcherName(MatcherName) {}
> +
> +  virtual VariantMatcher run(const SourceRange &NameRange,
> +                             ArrayRef<ParserValue> Args,
> +                             Diagnostics *Error) const {
> +    std::vector<VariantMatcher> InnerArgs;
> +    for (size_t i = 0, e = Args.size(); i != e; ++i) {
> +      const ParserValue &Arg = Args[i];
> +      const VariantValue &Value = Arg.Value;
> +      if (!Value.isMatcher()) {
> +        Error->addError(Arg.Range, Error->ET_RegistryWrongArgType)
> +            << (i + 1) << "Matcher<>" << Value.getTypeAsString();
> +        return VariantMatcher();
> +      }
> +      InnerArgs.push_back(Value.getMatcher());
> +    }
> +    return VariantMatcher::VariadicOperatorMatcher(Func, InnerArgs);
> +  }
> +
> +private:
> +  const VarFunc Func;
> +  const StringRef MatcherName;
> +};
> +
> +
>  /// Helper functions to select the appropriate marshaller functions.
>  /// They detect the number of arguments, arguments types and return type.
>
> @@ -410,6 +440,13 @@ AdaptativeOverloadCollector<ArgumentAdap
>    collect(typename FromTypeList::tail());
>  }
>
> +/// \brief Variadic operator overload.
> +MatcherCreateCallback *makeMatcherAutoMarshall(
> +    ast_matchers::internal::VariadicOperatorMatcherFunc Func,
> +    StringRef MatcherName) {
> +  return new VariadicOperatorMatcherCreateCallback(Func.Func, MatcherName);
> +}
> +
>  }  // namespace internal
>  }  // namespace dynamic
>  }  // namespace ast_matchers
>
> Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp?rev=189500&r1=189499&r2=189500&view=diff
> ==============================================================================
> --- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp (original)
> +++ cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp Wed Aug 28 13:42:04 2013
> @@ -77,9 +77,6 @@ RegistryMaps::RegistryMaps() {
>    //
>    // Polymorphic + argument overload:
>    // unless
> -  // eachOf
> -  // anyOf
> -  // allOf
>    // findAll
>    //
>    // Other:
> @@ -99,6 +96,8 @@ RegistryMaps::RegistryMaps() {
>
>    REGISTER_MATCHER(accessSpecDecl);
>    REGISTER_MATCHER(alignOfExpr);
> +  REGISTER_MATCHER(allOf);
> +  REGISTER_MATCHER(anyOf);
>    REGISTER_MATCHER(anything);
>    REGISTER_MATCHER(argumentCountIs);
>    REGISTER_MATCHER(arraySubscriptExpr);
> @@ -141,6 +140,7 @@ RegistryMaps::RegistryMaps() {
>    REGISTER_MATCHER(destructorDecl);
>    REGISTER_MATCHER(doStmt);
>    REGISTER_MATCHER(dynamicCastExpr);
> +  REGISTER_MATCHER(eachOf);
>    REGISTER_MATCHER(elaboratedType);
>    REGISTER_MATCHER(enumConstantDecl);
>    REGISTER_MATCHER(enumDecl);
>
> Modified: cfe/trunk/lib/ASTMatchers/Dynamic/VariantValue.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/VariantValue.cpp?rev=189500&r1=189499&r2=189500&view=diff
> ==============================================================================
> --- cfe/trunk/lib/ASTMatchers/Dynamic/VariantValue.cpp (original)
> +++ cfe/trunk/lib/ASTMatchers/Dynamic/VariantValue.cpp Wed Aug 28 13:42:04 2013
> @@ -38,13 +38,9 @@ public:
>          .str();
>    }
>
> -  virtual bool hasTypedMatcher(const MatcherOps &Ops) const {
> -    return Ops.canConstructFrom(*Matcher);
> -  }
> -
> -  virtual const DynTypedMatcher *getTypedMatcher(const MatcherOps &Ops) const {
> -    assert(hasTypedMatcher(Ops));
> -    return Matcher.get();
> +  virtual void makeTypedMatcher(MatcherOps &Ops) const {
> +    if (Ops.canConstructFrom(*Matcher))
> +      Ops.constructFrom(*Matcher);
>    }
>
>  private:
> @@ -80,25 +76,51 @@ public:
>      return (Twine("Matcher<") + Inner + ">").str();
>    }
>
> -  virtual bool hasTypedMatcher(const MatcherOps &Ops) const {
> -    return getTypedMatcher(Ops) != NULL;
> -  }
> -
> -  virtual const DynTypedMatcher *getTypedMatcher(const MatcherOps &Ops) const {
> -    const DynTypedMatcher* Found = NULL;
> +  virtual void makeTypedMatcher(MatcherOps &Ops) const {
> +    const DynTypedMatcher *Found = NULL;
>      for (size_t i = 0, e = Matchers.size(); i != e; ++i) {
>        if (Ops.canConstructFrom(*Matchers[i])) {
> -        if (Found) return NULL;
> +        if (Found)
> +          return;
>          Found = Matchers[i];
>        }
>      }
> -    return Found;
> +    if (Found)
> +      Ops.constructFrom(*Found);
>    }
>
> -private:
>    std::vector<const DynTypedMatcher *> Matchers;
>  };
>
> +class VariantMatcher::VariadicOpPayload : public VariantMatcher::Payload {
> +public:
> +  VariadicOpPayload(ast_matchers::internal::VariadicOperatorFunction Func,
> +                    ArrayRef<VariantMatcher> Args)
> +      : Func(Func), Args(Args) {}
> +
> +  virtual bool getSingleMatcher(const DynTypedMatcher *&Out) const {
> +    return false;
> +  }
> +
> +  virtual std::string getTypeAsString() const {
> +    std::string Inner;
> +    for (size_t i = 0, e = Args.size(); i != e; ++i) {
> +      if (i != 0)
> +        Inner += "&";
> +      Inner += Args[i].getTypeAsString();
> +    }
> +    return Inner;
> +  }
> +
> +  virtual void makeTypedMatcher(MatcherOps &Ops) const {
> +    Ops.constructVariadicOperator(Func, Args);
> +  }
> +
> +private:
> +  const ast_matchers::internal::VariadicOperatorFunction Func;
> +  const std::vector<VariantMatcher> Args;
> +};
> +
>  VariantMatcher::VariantMatcher() {}
>
>  VariantMatcher VariantMatcher::SingleMatcher(const DynTypedMatcher &Matcher) {
> @@ -110,6 +132,12 @@ VariantMatcher::PolymorphicMatcher(Array
>    return VariantMatcher(new PolymorphicPayload(Matchers));
>  }
>
> +VariantMatcher VariantMatcher::VariadicOperatorMatcher(
> +    ast_matchers::internal::VariadicOperatorFunction Func,
> +    ArrayRef<VariantMatcher> Args) {
> +  return VariantMatcher(new VariadicOpPayload(Func, Args));
> +}
> +
>  bool VariantMatcher::getSingleMatcher(const DynTypedMatcher *&Out) const {
>    if (Value) return Value->getSingleMatcher(Out);
>    return false;
>
> Modified: cfe/trunk/unittests/ASTMatchers/Dynamic/ParserTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/Dynamic/ParserTest.cpp?rev=189500&r1=189499&r2=189500&view=diff
> ==============================================================================
> --- cfe/trunk/unittests/ASTMatchers/Dynamic/ParserTest.cpp (original)
> +++ cfe/trunk/unittests/ASTMatchers/Dynamic/ParserTest.cpp Wed Aug 28 13:42:04 2013
> @@ -137,9 +137,9 @@ bool matchesRange(const SourceRange &Ran
>           Range.Start.Column == StartColumn && Range.End.Column == EndColumn;
>  }
>
> -const DynTypedMatcher *getSingleMatcher(const VariantValue &value) {
> +const DynTypedMatcher *getSingleMatcher(const VariantValue &Value) {
>    const DynTypedMatcher *Out;
> -  EXPECT_TRUE(value.getMatcher().getSingleMatcher(Out));
> +  EXPECT_TRUE(Value.getMatcher().getSingleMatcher(Out));
>    return Out;
>  }
>
>
> Modified: cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp?rev=189500&r1=189499&r2=189500&view=diff
> ==============================================================================
> --- cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp (original)
> +++ cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp Wed Aug 28 13:42:04 2013
> @@ -261,6 +261,33 @@ TEST_F(RegistryTest, Adaptative) {
>    EXPECT_FALSE(matches("void foo() { if (true) return; }", S));
>  }
>
> +TEST_F(RegistryTest, VariadicOp) {
> +  Matcher<Decl> D = constructMatcher(
> +      "anyOf", constructMatcher("recordDecl"),
> +      constructMatcher("namedDecl",
> +                       constructMatcher("hasName", std::string("foo"))))
> +      .getTypedMatcher<Decl>();
> +
> +  EXPECT_TRUE(matches("void foo(){}", D));
> +  EXPECT_TRUE(matches("struct Foo{};", D));
> +  EXPECT_FALSE(matches("int i = 0;", D));
> +
> +  D = constructMatcher(
> +      "allOf", constructMatcher("recordDecl"),
> +      constructMatcher(
> +          "namedDecl",
> +          constructMatcher("anyOf",
> +                           constructMatcher("hasName", std::string("Foo")),
> +                           constructMatcher("hasName", std::string("Bar")))))
> +      .getTypedMatcher<Decl>();
> +
> +  EXPECT_FALSE(matches("void foo(){}", D));
> +  EXPECT_TRUE(matches("struct Foo{};", D));
> +  EXPECT_FALSE(matches("int i = 0;", D));
> +  EXPECT_TRUE(matches("class Bar{};", D));
> +  EXPECT_FALSE(matches("class OtherBar{};", D));
> +}
> +
>  TEST_F(RegistryTest, Errors) {
>    // Incorrect argument count.
>    OwningPtr<Diagnostics> Error(new Diagnostics());
> @@ -285,6 +312,24 @@ TEST_F(RegistryTest, Errors) {
>    EXPECT_EQ("Incorrect type for arg 2. (Expected = Matcher<CXXRecordDecl>) != "
>              "(Actual = Matcher<FunctionDecl>)",
>              Error->toString());
> +
> +  // Bad argument type with variadic.
> +  Error.reset(new Diagnostics());
> +  EXPECT_TRUE(constructMatcher("anyOf", std::string(), Error.get()).isNull());
> +  EXPECT_EQ(
> +      "Incorrect type for arg 1. (Expected = Matcher<>) != (Actual = String)",
> +      Error->toString());
> +  Error.reset(new Diagnostics());
> +  EXPECT_TRUE(constructMatcher(
> +      "recordDecl",
> +      constructMatcher("allOf",
> +                       constructMatcher("isDerivedFrom", std::string("FOO")),
> +                       constructMatcher("isArrow")),
> +      Error.get()).isNull());
> +  EXPECT_EQ("Incorrect type for arg 1. "
> +            "(Expected = Matcher<CXXRecordDecl>) != "
> +            "(Actual = Matcher<CXXRecordDecl>&Matcher<MemberExpr>)",
> +            Error->toString());
>  }
>
>  } // end anonymous namespace
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits



More information about the cfe-commits mailing list