This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Wildcard expansion on Windows.
ClosedPublic

Authored by alexfh on Mar 22 2018, 5:29 AM.

Diff Detail

Event Timeline

alexfh created this revision.Mar 22 2018, 5:29 AM

Never seen this PURE_WINDOWS CMake variable. How is it different than MSVC?

PURE_WINDOWS is all Windows except Cygwin. It's an LLVM thing, not a CMake thing.

I don't know if setargv.obj is available on MinGW, and even if it is it's probably not gonna have the .obj extension, so this should probably be limited to MSVC.

rnk requested changes to this revision.Mar 22 2018, 3:13 PM
rnk added a subscriber: rnk.

Use llvm::sys::Process::GetArgumentVector, which already does wildcard expansion from what I can see. It works with Unicode command lines and isn't affected by locale.

This revision now requires changes to proceed.Mar 22 2018, 3:13 PM
In D44778#1046251, @rnk wrote:

Use llvm::sys::Process::GetArgumentVector, which already does wildcard expansion from what I can see. It works with Unicode command lines and isn't affected by locale.

I vaguely remember that windows console applications have to define wmain() instead of main() in order to work with unicode. Does llvm::sys::Process::GetArgumentVector allow to work around this?

In D44778#1046251, @rnk wrote:

Use llvm::sys::Process::GetArgumentVector, which already does wildcard expansion from what I can see. It works with Unicode command lines and isn't affected by locale.

I vaguely remember that windows console applications have to define wmain() instead of main() in order to work with unicode. Does llvm::sys::Process::GetArgumentVector allow to work around this?

Never mind. I looked at the windows implementation and see how =)

Thanks for the pointer. I'll update the patch.

alexfh updated this revision to Diff 139593.Mar 23 2018, 7:25 AM
  • Use llvm::sys::Process::GetArgumentVector.

I've not tested this on Windows, but I hope llvm::sys::Process::GetArgumentVector should have been tested by someone already.

rnk accepted this revision.Mar 23 2018, 10:58 AM

lgtm

tools/clang-format/ClangFormat.cpp
348 ↗(On Diff #139593)

s/couldn'g/couldn't/

This revision is now accepted and ready to land.Mar 23 2018, 10:58 AM
This revision was automatically updated to reflect the committed changes.