tbaeder added inline comments.

================
Comment at: clang/lib/AST/Interp/Interp.cpp:390
 
-  if (isa<RecordType>(ElemType.getTypePtr())) {
+  if (ElemType->isRecordType()) {
     const Record *R = BasePtr.getElemRecord();
----------------
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.


================
Comment at: clang/test/AST/Interp/cxx20.cpp:182
   };
   constexpr C3 c3; // expected-error {{must be initialized by a constant 
expression}} \
                    // expected-note {{subobject of type 'int' is not 
initialized}} \
----------------
This line actually needs the changes in this patch, the array is of record type.


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

Reply via email to