[PATCH] D45641: Rename sys::Process::GetArgumentVector -> sys::windows::GetCommandLineArguments

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 16 13:20:20 PDT 2018


ruiu added a comment.

In https://reviews.llvm.org/D45641#1068962, @stella.stamenova wrote:

> My concern with this change is that once GetArgumentVector no longer exists, any developer who wants to retrieve the arguments as UTF8 on all platforms will need to be aware that there is a windows-specific function needed to do so. By having GetArgumentVector in sys::Process, we have a unified way of retrieving arguments on all platforms and if in the future it was necessary to specialize on say, Android, the interface wouldn't have to change. I dislike the "GetArgumentVector" name because it is rather unclear what it does (and likely the reason why it was not uniformly used), but I do think it should stay in sys::process.


Does Android really need this functionality? As far as I know, this is actually Windows specific, and the only user of this functionality is InitLLVM at the moment (because I eliminated all the other uses.) The reason why Windows needs this is historical (this is developed in 90s), and it is hard to imagine that some new system does the same mistake again in the future. So I'm not interested in generalizing this to cover non-Windows system. If something is used only by one .cpp file, that feature doesn't (and perhaps shouldn't) live in llvm::sys namespace.


https://reviews.llvm.org/D45641





More information about the llvm-commits mailing list