zturner added inline comments.
================
Comment at: lib/Driver/Job.cpp:312
SmallVector<const char*, 128> Argv;
+ auto Envp = Environment.size() > 1
+ ? const_cast<const char **>(Environment.data())
----------------
Can you add `assert(Environment.back() == nullptr);`?
================
Comment at: lib/Driver/ToolChains/MSVC.cpp:478
+ size_t EnvBlockLen = 0;
+ while (EnvBlockWide[EnvBlockLen] != '\0') {
+ ++EnvCount;
----------------
`L'\0'`
================
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))
----------------
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)))
```
================
Comment at: lib/Driver/ToolChains/MSVC.cpp:492
+
+ Environment.reserve(EnvCount);
+
----------------
`EnvCount+1`, for the extra null terminator.
================
Comment at: lib/Driver/ToolChains/MSVC.cpp:497
+ // find it.
+ for (const char *Cursor = EnvBlock.data(); *Cursor != '\0';) {
+ llvm::StringRef EnvVar(Cursor);
----------------
We could avoid the manual index and loop counters if we do something like this:
```
StringRef Cursor(EnvBlock);
while (!Cursor.empty()) {
StringRef ThisVar;
std::tie(ThisVar, Cursor) = Cursor.split('\0');
}
```
================
Comment at: lib/Driver/ToolChains/MSVC.cpp:499
+ llvm::StringRef EnvVar(Cursor);
+ if (EnvVar.startswith_lower("path=")) {
+ using SubDirectoryType = toolchains::MSVCToolChain::SubDirectoryType;
----------------
Too bad `StringRef` doesn't have `consume_front_lower()`. That would have been
perfect here.
================
Comment at: lib/Driver/ToolChains/MSVC.cpp:517
+ }
+ }
+ SkipSettingEnvironment:
----------------
I think you need to push 1 more null terminator onto the end here to terminate
the block.
https://reviews.llvm.org/D30991
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
