[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 🔍

Tue May 21 21:00:54 PDT 2019

stephanemoore planned changes to this revision.
stephanemoore added a comment.

> Out of curiosity, how invasive is Option 2 within our own code base?

I am still working on getting something working but I don't anticipate anything notably invasive.

> Does that option require fixing a lot of code?

I believe it doesn't require fixing a lot of code within our codebase. I think we just need to fix the overloaded `isDerivedFrom` matcher and both `isSameOrDerivedFrom` matchers.

> Are the breakages loud or silent?

I expect that the breakages should be loud compilation failures.

> Is the code easy to modify, or are there corner cases where this option becomes problematic?

I am still working on putting together something that works. I am still getting familiar with polymorphic matchers so I probably can't give a reliable assessment of the ease of the code changes just yet. I imagine that the changes in this commit to convert the overloaded `isDerivedFrom` matcher and the two `isSameOrDerivedFrom` matchers could be demonstrative to others as to how to resolve breakages. I am not confident enough to state that with certainty though.

> I prefer Option 2 because it is a cleaner, more understandable design for the matchers. If it turns out that this option causes a hard break (rather than silently changing matcher behavior) and the changes needed to get old code to compile again are minimal, I think it may be a reasonable choice.

I agree that Option 2 is preferable if we are allowed to require a non-zero amount of migration work from downstream clients. I will continue working on Option 2 and will return once I have something working.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60543/new/

https://reviews.llvm.org/D60543