aaron.ballman added a comment.
In D60543#1497576 <https://reviews.llvm.org/D60543#1497576>, @stephanemoore
wrote:
> I did some digging and I believe there are two approaches that we can take to
> extend `isDerivedFrom` to support Objective-C classes.
>
> **Option 1: Match on Common Ancestor Declaration Type**:
> Convert `isDerivedFrom` to match on the common ancestor declaration type,
> `NamedDecl`, and return `false` for
> declaration types other than `CXXRecordDecl` and `ObjCInterfaceDecl`.
>
> Advantages:
> • Largely works in-place without requiring major changes to matchers built
> on top of `isDerivedFrom`.
>
> Disadvantages:
> • Allows nonsensical matchers, e.g., `functionDecl(isDerivedFrom("X"))`.
>
> **Option 2: Convert to Polymorphic Matcher**:
> Convert `isDerivedFrom` to a polymorphic matcher supporting `CXXRecordDecl`
> and `ObjCInterfaceDecl`.
>
> Advantages:
> • Restricts usage of `isDerivedFrom` to `CXXRecordDecl` and
> `ObjCInterfaceDecl`.
>
> Disadvantages:
> • Potentially requires many or all matchers using `isDerivedFrom` to
> refactor to adapt to the polymorphic matcher interface.
>
> **Evaluation**
> I have been going back and forth as to which approach is superior. Option 1
> promotes source stability at the cost of usability while
> option 2 seems to promote usability at the cost of source stability. I
> exported a portrayal of option 1 for consideration as it
> required less invasive changes. I can see arguments supporting both
> approaches.
>
> What do you think? Which of the two approaches do you think we should go
> with? Is there another approach that I have not thought of?
This is a great breakdown, thank you for providing it!
Out of curiosity, how invasive is Option 2 within our own code base? Does that
option require fixing a lot of code? Are the breakages loud or silent? Is the
code easy to modify, or are there corner cases where this option becomes
problematic? 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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60543/new/
https://reviews.llvm.org/D60543
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
