[PATCH] D33383: [GSoC] Flag value completion for clang
Vassil Vassilev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 16 09:11:39 PDT 2017
v.g.vassilev added inline comments.
================
Comment at: clang/include/clang/Driver/Options.td:2198
def stdlib_EQ : Joined<["-", "--"], "stdlib=">, Flags<[CC1Option]>,
- HelpText<"C++ standard library to use">;
+ HelpText<"C++ standard library to use">, Values<"libc++,libstdc++,platform">;
def sub__library : JoinedOrSeparate<["-"], "sub_library">;
----------------
ruiu wrote:
> v.g.vassilev wrote:
> > `Values` seems too generic, can we use `ArgValues` instead?
> I'd keep it as `Values`, as everything is essentially related to command line arguments, and `Args` seems redundant. For example, we didn't name `ArgFlags` but just `Flags`, and `HelpText` instead of `ArgHelpText`, etc.
My reasoning for asking this is that I wanted to hint about the relationship between the values (as this is a very broad term) and arguments. I'd read `ArgValues` as possible values for an argument without having to dig into context.
That said, I don't think switching back to `Values` is a significant issue, if @ruiu feels strongly about it , please follow his suggestion and land the patch.
================
Comment at: clang/lib/Driver/Driver.cpp:1226
+ // So spilit them with "," in order to make list of flags.
+ PassedFlags.split(ListOfPassedFlags, ",", -1, false);
+ std::vector<std::string> SuggestedCompletions = Opts->findByPrefix(ListOfPassedFlags[0]);
----------------
ruiu wrote:
> You are using `PassedFlags` only once. Do you want to replace it with `StringRef(A->getValue()).split(...)`?
`PassedFlags` looks more readable to casual readers like me ;)
https://reviews.llvm.org/D33383
More information about the llvm-commits
mailing list