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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to