On Thu, Sep 20, 2012 at 5:49 PM, David Blaikie <[email protected]> wrote: > On Thu, Sep 20, 2012 at 1:49 AM, Daniel Jasper > <[email protected]> wrote: >> Hi klimek, >> >> By changing the conversion operator into a conversion constructor, we can >> enabled based on the template parameters leading to better error messages. >> E.g.: stmt(decl()) will now create an error message including: >> >> note: candidate function not viable: no known conversion from >> 'clang::ast_matchers::internal::BindableMatcher<clang::Decl>' to 'const >> clang::ast_matchers::internal::Matcher<clang::Stmt>' for 1st argument, > > I'm curious - what was the error message before? (I wonder if Clang > could do a better job at it so that this API choice doesn't adversely > affect usability)
Previously the compilation would break at instantiation time where matcher.matches(node) was called, and there wouldn't be a function 'matches' of the required type. At which point, as a user, my thinking is: "matches? I didn't call no matches?" Makes sense? Cheers, /Manuel > >> >> >> http://llvm-reviews.chandlerc.com/D44 >> >> Files: >> include/clang/ASTMatchers/ASTMatchersInternal.h >> >> Index: include/clang/ASTMatchers/ASTMatchersInternal.h >> =================================================================== >> --- include/clang/ASTMatchers/ASTMatchersInternal.h >> +++ include/clang/ASTMatchers/ASTMatchersInternal.h >> @@ -257,21 +257,23 @@ >> explicit Matcher(MatcherInterface<T> *Implementation) >> : Implementation(Implementation) {} >> >> + /// \brief Implicitly converts \c Other to a Matcher<T>. >> + /// >> + /// Requires \c T to be derived from \c From. >> + template <typename From> >> + Matcher(const Matcher<From> &Other, >> + typename llvm::enable_if_c< >> + llvm::is_base_of<From, T>::value && >> + !llvm::is_same<From, T>::value >::type* = 0) >> + : Implementation(new ImplicitCastMatcher<From>(Other)) {} >> + >> /// \brief Forwards the call to the underlying MatcherInterface<T> >> pointer. >> bool matches(const T &Node, >> ASTMatchFinder *Finder, >> BoundNodesTreeBuilder *Builder) const { >> return Implementation->matches(Node, Finder, Builder); >> } >> >> - /// \brief Implicitly converts this object to a Matcher<Derived>. >> - /// >> - /// Requires Derived to be derived from T. >> - template <typename Derived> >> - operator Matcher<Derived>() const { >> - return Matcher<Derived>(new ImplicitCastMatcher<Derived>(*this)); >> - } >> - >> /// \brief Returns an ID that uniquely identifies the matcher. >> uint64_t getID() const { >> /// FIXME: Document the requirements this imposes on matcher >> @@ -289,22 +291,22 @@ >> } >> >> private: >> - /// \brief Allows conversion from Matcher<T> to Matcher<Derived> if >> Derived >> - /// is derived from T. >> - template <typename Derived> >> - class ImplicitCastMatcher : public MatcherInterface<Derived> { >> + /// \brief Allows conversion from Matcher<Base> to Matcher<T> if T >> + /// is derived from Base. >> + template <typename Base> >> + class ImplicitCastMatcher : public MatcherInterface<T> { >> public: >> - explicit ImplicitCastMatcher(const Matcher<T> &From) >> + explicit ImplicitCastMatcher(const Matcher<Base> &From) >> : From(From) {} >> >> - virtual bool matches(const Derived &Node, >> + virtual bool matches(const T &Node, >> ASTMatchFinder *Finder, >> BoundNodesTreeBuilder *Builder) const { >> return From.matches(Node, Finder, Builder); >> } >> >> private: >> - const Matcher<T> From; >> + const Matcher<Base> From; >> }; >> >> llvm::IntrusiveRefCntPtr< MatcherInterface<T> > Implementation; >> >> _______________________________________________ >> 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
