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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1133-1134
+  } else if (const auto *SL = dyn_cast<StringLiteral>(Initializer)) {
+    const ArrayType *AT = SL->getType()->getAsArrayTypeUnsafe();
+    const auto *CAT = cast<ConstantArrayType>(AT);
+    size_t NumElems = CAT->getSize().getZExtValue();
----------------



================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1139
+    //   and Porgram::createGlobalString().
+    const size_t CharWidth = SL->getCharByteWidth();
+    PrimType CharType;
----------------



================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1143
+    case 1:
+      CharType = PT_Sint8;
+      break;
----------------
Should we be looking at the sign of `char` to decide whether to use a uint8 or an sint8?


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1157
+    for (size_t I = 0; I != NumElems; ++I) {
+      const uint32_t CodePoint = I < N ? SL->getCodeUnit(I) : 0;
+      // TODO(Perf): 0 is implicit; we can just stop iterating at that point.
----------------



================
Comment at: clang/test/AST/Interp/literals.cpp:354-359
+  constexpr char foo[12] = "abc";
+  static_assert(foo[0] == 'a', "");
+  static_assert(foo[1] == 'b', "");
+  static_assert(foo[2] == 'c', "");
+  static_assert(foo[3] == 0, "");
+  static_assert(foo[11] == 0, "");
----------------
I'd like to see some tests for the other encodings, as well as a test with embedded null characters in the literal.

Testing a string literal that's longer than the array is something we should think about. That code is ill-formed in C++, so I don't think we can add a test for it yet, but it's only a warning in C.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137488