aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LG with a request for an additional test.
================
Comment at: clang/lib/AST/Interp/Interp.cpp:390
- if (isa<RecordType>(ElemType.getTypePtr())) {
+ if (ElemType->isRecordType()) {
const Record *R = BasePtr.getElemRecord();
----------------
tbaeder wrote:
> shafik wrote:
> > aaron.ballman wrote:
> > > The difference between these two is that `isRecordType()` is looking at
> > > the canonical type whereas `isa<>` is looking at the type under
> > > inspection rather than the canonical type. I'd expect these to have the
> > > same behavior in most cases, but only matter for cases involving typedefs.
> > >
> > > I think you're correct about the test case below not needing these
> > > particular changes -- at least, I'm not seeing what's changed that should
> > > impact the test. Should this be split into two changes? 1) Expose the
> > > test, 2) Make this functional change + add a new test where the canonical
> > > type is different to demonstrate the fix.
> > +1
> Can you come up with a small test case that would show the difference? You
> mentioned typedefs, but if the array is of a typedef type, the old `isa<>`
> version doesn't work either.
Ah, I see what you mean, yeah. With a typedef type, the `isa` would have also
failed. That's a good test case to add to this patch as well.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143334/new/
https://reviews.llvm.org/D143334
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
