[cfe-commits] [PATCH] Change counter-intuitive behavior of isDerivedFrom

[Chandler Carruth]

On Fri, Sep 7, 2012 at 8:06 AM, Daniel Jasper <
reviews at llvm-reviews.chandlerc.com> wrote:

> Hi klimek,
>
> Change the behavior of the isDerivedFrom-matcher to not match on the class
> itself. This caused some confusion (intuitively, a class is not derived
> from itself) and makes it hard to write certain matchers, e.g. "match and
> bind any pair of base and subclass".
>
> The original behavior can be achieved with a new isA-matcher.  Similar to
> all other matchers, this matcher has the same behavior and name as the
> corresponding AST-entity - in this case the isa<>() function.
>

This is not a new idea. In fact, when we first built the isDerivedFrom
matcher over a year ago Manuel and others argued for what you propose. I
argued firmly against it, and my feelings have not changed. Manuel should
have remembered that.

See Sebastien Redl's comment for why I firmly believe that the current
behavior is the correct behavior: std::is_base_of behaves the exact same
way.

I don't see why "math and bind any pair of base and subclass" is that hard
to write. It only requires a not matcher that tests for type equality. Can
you give an example that is hard to write, and maybe there is a better way
to write it?

I also strongly object to the name "isA" as a matcher. We already have an
"isa<..>()" utility within LLVM and Clang that is extremely widely used. I
*do not want* a case-difference to select between the two in any
circumstance. And again, I raised this objection when the idea was first
proposed over a year ago.

So, let's discuss the actual problems with writing matchers, and see if we
can find solutions for them?

-Chandler

PS: Just to be clear, I claim that the AST matchers are currently in the
exact same situation as any other C++ interfaces in Clang. While we should
not gratuitously break out-of-tree code, we should continue to feel free to
change the API in backwards incompatible ways, and document these changes
to help people with out-of-tree code update to match. So, I'm not worried
about that aspect of this patch. =]
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120907/2a1c2d2a/attachment.html>