[PATCH] D136090: Handle errors in expansion of response files

Fri Oct 28 05:19:18 PDT 2022

mgorny accepted this revision.
mgorny added a comment.
This revision is now accepted and ready to land.

LGTM

================
Comment at: clang/unittests/Driver/ToolChainTest.cpp:604-606
+  char *StrBuff = (char *)Alloc.Allocate(6, 2);
+  std::memcpy(StrBuff, "\xFF\xFE\x00\xD8\x00\x00", 6);
+  StringRef BadUTF(StrBuff, 6);
----------------
sepavloff wrote:
> mgorny wrote:
> > Wouldn't it be possible to use `std::string`, or maybe even `std::array<char, ...>` here? I think it'd be less error-prone.
> Nor `std::string` neither `std::array<char,N>` provide custom alignment.
> 
> UTF-16 string is constructed here from array of chars to avoid problems with endianness. It consists of 2-byte elements and is expected to be aligned on 2-byte boundary. Array of chars is aligned on byte and sometimes the test failed due to invalid alignment.
> 
> So we have to use bare pointer to have guaranteed alignment.
> 
Ah. Could you leave a comment to make the alignment requirements clear? I'm a bit surprised that we have to meet alignment requirements when "reading" a file.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136090