tahonermann accepted this revision.
tahonermann added a comment.
This revision is now accepted and ready to land.
This looks good to me now. I'm sorry for taking so long to return to review.
================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1098-1099
+
+ unsigned N = SL->getLength();
+ for (size_t I = 0; I != NumElems; ++I) {
+ uint32_t CodePoint = I < N ? SL->getCodeUnit(I) : 0;
----------------
tbaeder wrote:
> tbaeder wrote:
> > tahonermann wrote:
> > > tahonermann wrote:
> > > > Aren't `N` and `NumElems` guaranteed to have the same value here? Both
> > > > are derived from `SL`. The code seems to be written with the
> > > > expectation that `NumElems` corresponds to the number of elements to be
> > > > iniitialized in the target array.
> > > I see the change to now use the minimum of `SL->getLength()` and
> > > `CAT->getSize().getZExtValue()`. Based on https://godbolt.org/z/5sTWExTac
> > > this looks to be unnecessary. When a string literal is used as an array
> > > initializer, it appears that the type of the string literal is adjusted
> > > to match the size of the array being initialized. I suggest using only
> > > `CAT->getSize().getZExtValue()` and adding a comment that this code
> > > depends on that adjustment.
> > That is good to know and makes sense, thanks!
> That actually doesn't work. They type might be adjusted, but `getCodeUnit()`
> still asserts that the index is `< getLength()`. :(
Ah, ok, that makes sense, thanks. I agree this is the right approach for
enumerating just the code units in the string literal that are used to
initialize the target now.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137488/new/
https://reviews.llvm.org/D137488
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
