hamzasood marked an inline comment as done. hamzasood added inline comments.
================ Comment at: lib/Driver/ToolChains/MSVC.cpp:487-488 + if (!llvm::convertUTF16ToUTF8String( + llvm::ArrayRef<char>(reinterpret_cast<char *>(EnvBlockWide.get()), + EnvBlockLen * sizeof(EnvBlockWide[0])), + EnvBlock)) ---------------- zturner wrote: > There's an overload of `convertUTF16ToUTF8String` that takes an > `ArrayRef<UTF16>`. So I think you can just write this: > > ``` > if (!llvm::convertUTF16ToUTF8String(makeArrayRef(EnvBlockWide.get(), > EnvBlockLen))) > ``` Using that overload would involve casting wchar_t* to UTF16* (i.e. unsigned char*), which I think breaks aliasing rules. ================ Comment at: lib/Driver/ToolChains/MSVC.cpp:517 + } + } + SkipSettingEnvironment: ---------------- zturner wrote: > zturner wrote: > > I think you need to push 1 more null terminator onto the end here to > > terminate the block. > Actually if you use the `std::tie()` algorithm I proposed, then it will enter > the body of the loop on the terminating null and push it back (as long as > `MakeArgString` returns `nullptr` when its argument is empty, which I haven't > checked) Command::setEnvironment adds a nullptr onto the end of any vector it's given. I figured it's simpler to do it there than to rely on the caller. Do you think that should be changed? https://reviews.llvm.org/D30991 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
