[PATCH] D137488: [clang][Interp] Array initialization via string literal

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.


================
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:
> 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()`. :(


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137488/new/

https://reviews.llvm.org/D137488