[PATCH] D33383: [GSoC] Flag value completion for clang

Rui Ueyama via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 14 11:28:37 PDT 2017


ruiu added a comment.

Overall looking good. Good work! A few minor 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">;
----------------
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.


================
Comment at: clang/lib/Driver/Driver.cpp:1224
+    SmallVector<StringRef, 8> ListOfPassedFlags;
+    // PassedFlags are incomplete flags which format is "c,=,-std,-fsyntax-only"
+    // So spilit them with "," in order to make list of flags.
----------------
nit: insert a blank line before a meaningful code block so that code doesn't look too dense.


================
Comment at: clang/lib/Driver/Driver.cpp:1225
+    // PassedFlags are incomplete flags which format is "c,=,-std,-fsyntax-only"
+    // So spilit them with "," in order to make list of flags.
+    PassedFlags.split(ListOfPassedFlags, ",", -1, false);
----------------
make a list of flag values.


================
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]);
----------------
You are using `PassedFlags` only once. Do you want to replace it with `StringRef(A->getValue()).split(...)`?


================
Comment at: clang/test/Driver/autocomplete.c:7
 // NONE: foo
+// RUN: %clang --autocomplete=l,=,-stdlib | FileCheck %s -check-prefix=STDLIB
+// STDLIB: libc++ libstdc++
----------------
Why do you want to pass the arguments in the reverse order?


https://reviews.llvm.org/D33383





More information about the cfe-commits mailing list