[PATCH] D47578: Do not enforce absolute path argv0 in windows

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

It seems like you are trying a bit too hard to keep the original code which is not always a good idea to keep the clarity of the code.

So, IIUC, you this function:

- returns a full filename of the current executable. That can be written using GetModuleFileName and GetLongPathName

then you basically do the following to

  sys::path::remove_filename(Arg0);
  sys::path::append(Arg0, sys::path::filename(ExectuablePath));
  




================
Comment at: llvm/lib/Support/Windows/Process.inc:211
 
 static std::error_code ExpandShortFileName(const wchar_t *Arg,
+                                           SmallVectorImpl<wchar_t> &LongPath) {
----------------
This function is used only by your new function, so it is probably better to inline it.


================
Comment at: llvm/lib/Support/Windows/Process.inc:226
+
+static std::error_code GetLongArgv0FullPath(const wchar_t *Argv0,
+                                            SmallVectorImpl<wchar_t> &LongArgv0) {
----------------
You can always ignore Argv0, no? Since GetModuleFileName is always available, you probably should use it unconditionally, ignoring the original argv[0].


================
Comment at: llvm/lib/Support/Windows/Process.inc:251
+  // This may change argv0 like below,
+  // * clang -> clang.exe (just add extension)
+  // * CLANG_~1.EXE -> clang++.exe (extend shorname)
----------------
I believe GetModuleFileName always returns a path with an extension, though it might be 8.3 path.


================
Comment at: llvm/lib/Support/Windows/Process.inc:268-269
+
+  return windows::UTF8ToUTF16(StringRef(UTF8Argv0.data(), UTF8Argv0.size()),
+                              LongArgv0);
 }
----------------
Don't convert utf-8 to utf-16 only to convert it back to utf-8 immediately after returning from this function.


https://reviews.llvm.org/D47578