[cfe-commits] [PATCH] Provide better error messages for incorrect matchers

[David Blaikie]

On Thu, Sep 20, 2012 at 1:49 AM, Daniel Jasper
<reviews at llvm-reviews.chandlerc.com> 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)

>
>
> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>