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

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi closed this revision.
yamaguchi added a comment.

Landed in https://reviews.llvm.org/rL305805.


https://reviews.llvm.org/D33383



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added inline comments.



Comment at: clang/include/clang/Driver/Options.td:496
 def cl_std_EQ : Joined<["-"], "cl-std=">, Group, 
Flags<[CC1Option]>,
-  HelpText<"OpenCL language standard to compile for.">;
+  HelpText<"OpenCL language standard to compile for.">, 
Values<"cl,CL,cl1.1,CL1.1,cl1.2,CL1.2,cl2.0,CL2.0">;
 def cl_denorms_are_zero : Flag<["-"], "cl-denorms-are-zero">, 
Group, Flags<[CC1Option]>,

v.g.vassilev wrote:
> I think we should have a way to express that the values are case insensitive, 
> instead of enumerating all possible combinations.
Looks like this is not really a case-insensitive option, as it does not allow 
"cL2.0" for example. The enumerated values are exact list of values that the 
option accepts.


https://reviews.llvm.org/D33383



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Rui Ueyama via Phabricator via cfe-commits]

ruiu accepted this revision.
ruiu added a comment.

LGTM


https://reviews.llvm.org/D33383



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Raphael Isemann via Phabricator via cfe-commits]

teemperor accepted this revision.
teemperor added a comment.

LGTM.


https://reviews.llvm.org/D33383



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

LGTM. My comments can be addressed in a separate patch.




Comment at: clang/include/clang/Driver/Options.td:496
 def cl_std_EQ : Joined<["-"], "cl-std=">, Group, 
Flags<[CC1Option]>,
-  HelpText<"OpenCL language standard to compile for.">;
+  HelpText<"OpenCL language standard to compile for.">, 
Values<"cl,CL,cl1.1,CL1.1,cl1.2,CL1.2,cl2.0,CL2.0">;
 def cl_denorms_are_zero : Flag<["-"], "cl-denorms-are-zero">, 
Group, Flags<[CC1Option]>,

I think we should have a way to express that the values are case insensitive, 
instead of enumerating all possible combinations.



Comment at: clang/include/clang/Driver/Options.td:1341
   HelpText<"Which overload candidates to show when overload resolution fails: "
-   "best|all; defaults to all">;
+   "best|all; defaults to all">, Values<"best,all">;
 def fshow_column : Flag<["-"], "fshow-column">, Group, 
Flags<[CC1Option]>;

We should be able to express which value is the default value if nothing is 
specified. A non-very readable solution would be to put the default always as 
the first value.


https://reviews.llvm.org/D33383



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi updated this revision to Diff 102982.
yamaguchi added a comment.

Add support and test for 
-cl-std=,-fno-sanitize-coverage=,-ffp-contract=,-flto=,-fveclib=,-fshow-overloads=,-fvisibility=,-mfloat-abi=,-mthread-model.


https://reviews.llvm.org/D33383

Files:
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/DriverOptions.cpp
  clang/test/Driver/autocomplete.c
  clang/utils/bash-autocomplete.sh
  lld/COFF/Driver.h
  lld/COFF/DriverUtils.cpp
  lld/ELF/Driver.h
  lld/ELF/DriverUtils.cpp
  lld/lib/Driver/DarwinLdDriver.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/Option/OptTable.cpp
  llvm/lib/Option/Option.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -196,6 +196,9 @@
   OS << ", nullptr";
 
 // The option meta-variable name (unused).
+OS << ", nullptr";
+
+// The option Values (unused for groups).
 OS << ", nullptr)\n";
   }
   OS << "\n";
@@ -285,6 +288,13 @@
 else
   OS << "nullptr";
 
+// The option Values. Used for shell autocompletion.
+OS << ", ";
+if (!isa(R.getValueInit("Values")))
+  write_cstring(OS, R.getValueAsString("Values"));
+else
+  OS << "nullptr";
+
 OS << ")\n";
   }
   OS << "#endif // OPTION\n";
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -18,8 +18,9 @@
 
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-   HELPTEXT, METAVAR) OPT_##ID,
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, VALUES)  \
+  OPT_##ID,
 #include "Opts.inc"
   LastOption
 #undef OPTION
@@ -36,10 +37,10 @@
 };
 
 static const OptTable::Info InfoTable[] = {
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-   HELPTEXT, METAVAR)   \
-  { PREFIX, NAME, HELPTEXT, METAVAR, OPT_##ID, Option::KIND##Class, PARAM, \
-FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS },
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, VALUES)  \
+  {PREFIX, NAME,  HELPTEXT,METAVAR, OPT_##ID,  Option::KIND##Class,\
+   PARAM,  FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS, VALUES},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/tools/llvm-cvtres/llvm-cvtres.cpp
===
--- llvm/tools/llvm-cvtres/llvm-cvtres.cpp
+++ llvm/tools/llvm-cvtres/llvm-cvtres.cpp
@@ -37,7 +37,7 @@
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR)  \
+   HELPTEXT, METAVAR, VALUES)  \
   OPT_##ID,
 #include "Opts.inc"
 #undef OPTION
@@ -49,12 +49,12 @@
 
 static const opt::OptTable::Info InfoTable[] = {
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR)  \
+   HELPTEXT, METAVAR, VALUES)  \
   {\
-  PREFIX,  NAME, HELPTEXT, \
-  METAVAR, OPT_##ID, opt::Option::KIND##Class, \
-  PARAM,   FLAGS,OPT_##GROUP,  \
-  OPT_##ALIAS, ALIASARGS},
+  PREFIX,  NAME,  HELPTEXT,\
+  METAVAR, OPT_##ID,  opt::Option::KIND##Class,\
+  PARAM,   FLAGS, OPT_##GROUP, \
+  OPT_##ALIAS, ALIASARGS, VALUES},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
===
--- llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
+++ llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
@@ -31,7 +31,7 @@
 
 enum {
   OPT_INVALID = 0,
-#define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11) OPT_##ID,
+#define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11, 

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

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi updated this revision to Diff 102981.
yamaguchi marked 6 inline comments as done.
yamaguchi added a comment.

Update code comment according to Rui's comment.


https://reviews.llvm.org/D33383

Files:
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/DriverOptions.cpp
  clang/test/Driver/autocomplete.c
  clang/utils/bash-autocomplete.sh
  lld/COFF/Driver.h
  lld/COFF/DriverUtils.cpp
  lld/ELF/Driver.h
  lld/ELF/DriverUtils.cpp
  lld/lib/Driver/DarwinLdDriver.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/Option/OptTable.cpp
  llvm/lib/Option/Option.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -196,6 +196,9 @@
   OS << ", nullptr";
 
 // The option meta-variable name (unused).
+OS << ", nullptr";
+
+// The option Values (unused for groups).
 OS << ", nullptr)\n";
   }
   OS << "\n";
@@ -285,6 +288,13 @@
 else
   OS << "nullptr";
 
+// The option Values. Used for shell autocompletion.
+OS << ", ";
+if (!isa(R.getValueInit("Values")))
+  write_cstring(OS, R.getValueAsString("Values"));
+else
+  OS << "nullptr";
+
 OS << ")\n";
   }
   OS << "#endif // OPTION\n";
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -18,8 +18,9 @@
 
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-   HELPTEXT, METAVAR) OPT_##ID,
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, VALUES)  \
+  OPT_##ID,
 #include "Opts.inc"
   LastOption
 #undef OPTION
@@ -36,10 +37,10 @@
 };
 
 static const OptTable::Info InfoTable[] = {
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-   HELPTEXT, METAVAR)   \
-  { PREFIX, NAME, HELPTEXT, METAVAR, OPT_##ID, Option::KIND##Class, PARAM, \
-FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS },
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, VALUES)  \
+  {PREFIX, NAME,  HELPTEXT,METAVAR, OPT_##ID,  Option::KIND##Class,\
+   PARAM,  FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS, VALUES},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/tools/llvm-cvtres/llvm-cvtres.cpp
===
--- llvm/tools/llvm-cvtres/llvm-cvtres.cpp
+++ llvm/tools/llvm-cvtres/llvm-cvtres.cpp
@@ -37,7 +37,7 @@
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR)  \
+   HELPTEXT, METAVAR, VALUES)  \
   OPT_##ID,
 #include "Opts.inc"
 #undef OPTION
@@ -49,12 +49,12 @@
 
 static const opt::OptTable::Info InfoTable[] = {
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR)  \
+   HELPTEXT, METAVAR, VALUES)  \
   {\
-  PREFIX,  NAME, HELPTEXT, \
-  METAVAR, OPT_##ID, opt::Option::KIND##Class, \
-  PARAM,   FLAGS,OPT_##GROUP,  \
-  OPT_##ALIAS, ALIASARGS},
+  PREFIX,  NAME,  HELPTEXT,\
+  METAVAR, OPT_##ID,  opt::Option::KIND##Class,\
+  PARAM,   FLAGS, OPT_##GROUP, \
+  OPT_##ALIAS, ALIASARGS, VALUES},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
===
--- llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
+++ llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
@@ -31,7 +31,7 @@
 
 enum {
   OPT_INVALID = 0,
-#define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11) OPT_##ID,
+#define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11, _12) OPT_##ID,
 #include "Options.inc"
 #undef OPTION
 };
@@

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

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

Yuka,

This is beautiful. Overall looking pretty good. Some minor stylistic comments.




Comment at: clang/include/clang/Driver/Options.h:43
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  
\
+   HELPTEXT, METAVAR, ARGVALUE)
\
+  OPT_##ID,

Since you are now using `Values` as a variable name, you want to rename 
ARGVALUE VALUES.



Comment at: clang/include/clang/Driver/Options.td:1709-1710
   HelpText<"The thread model to use, e.g. posix, single (posix by default)">;
-def meabi : Separate<["-"], "meabi">, Group, Flags<[CC1Option]>,
+def meabi : Separate<["-"], "meabi">, Group, Flags<[CC1Option]>, 
Values<"default,4,5,gnu">,
   HelpText<"Set EABI type, e.g. 4, 5 or gnu (default depends on triple)">;
 

You wrote `Values` before `HelpText` here but in the opposite order for 
stdlib_EQ. This is not a strict rule or anything, but it is better to be 
consistent in the option order.



Comment at: clang/lib/Driver/Driver.cpp:1233
+
+// When the flag was passed like "--autocomplete=-fsyn"
+// we want to autocomplete "-fsyn" as "-fsyntax-only"

Move this inside the first `if` so that it is clear that this comment does not 
describe the entire `if` and `else` but only the `if` part.

You could improve the comment: If the flag is in the form of 
"--autocomplete=-foo", we were requested to print out all option names that 
start with "-foo". For example, "--autocomplete=-fsyn" is expanded to 
"-fsyntax-only".



Comment at: clang/lib/Driver/Driver.cpp:1238
+} else {
+  // When flags were passed like "--autocomplete=-stdlib=,l"
+  // we want to autocomplete appropriate value which starts with "l"

Likewise: If the flag is in the form of "--autocomplete=-foo,bar", we were 
requested to print out all option values for "-foo" that start with "bar". For 
example, "--autocomplete=-stdlib=,l" is expanded to "libc++" and "libstdc++".



Comment at: clang/utils/bash-autocomplete.sh:7
 
-  flags=$( clang --autocomplete="$cur" )
-  if [[ "$flags" == "" || "$cur" == "" ]]; then
+  local w1="${COMP_WORDS[$cword - 1]}"
+  local w2="${COMP_WORDS[$cword - 2]}"

It is better to describe what you are doing here and why. How about this:

bash autocompletion always separates '=' as a token even if there's no space 
before/after it. On the other hand, '=' is just a regular character for clang 
options that contain '='. For example, "-stdlib=" is defined as is, instead of 
"-stdlib" and "=". So, we need to partially undo bash tokenization here for 
impedance matching.



Comment at: llvm/lib/Option/OptTable.cpp:189
 
+// returns true if one of the Prefixes + In.Names matches Option
+static bool optionMatches(const OptTable::Info &In, StringRef Option) {

returns -> Returns


https://reviews.llvm.org/D33383



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi updated this revision to Diff 102964.
yamaguchi added a comment.

Made a trivial change in optionMatches.


https://reviews.llvm.org/D33383

Files:
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/DriverOptions.cpp
  clang/test/Driver/autocomplete.c
  clang/utils/bash-autocomplete.sh
  lld/COFF/Driver.h
  lld/COFF/DriverUtils.cpp
  lld/ELF/Driver.h
  lld/ELF/DriverUtils.cpp
  lld/lib/Driver/DarwinLdDriver.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/Option/OptTable.cpp
  llvm/lib/Option/Option.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -196,6 +196,9 @@
   OS << ", nullptr";
 
 // The option meta-variable name (unused).
+OS << ", nullptr";
+
+// The option Values (unused for groups).
 OS << ", nullptr)\n";
   }
   OS << "\n";
@@ -285,6 +288,13 @@
 else
   OS << "nullptr";
 
+// The option Values. Used for shell autocompletion.
+OS << ", ";
+if (!isa(R.getValueInit("Values")))
+  write_cstring(OS, R.getValueAsString("Values"));
+else
+  OS << "nullptr";
+
 OS << ")\n";
   }
   OS << "#endif // OPTION\n";
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -18,8 +18,9 @@
 
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-   HELPTEXT, METAVAR) OPT_##ID,
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, ARGVALUE)\
+  OPT_##ID,
 #include "Opts.inc"
   LastOption
 #undef OPTION
@@ -36,10 +37,10 @@
 };
 
 static const OptTable::Info InfoTable[] = {
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-   HELPTEXT, METAVAR)   \
-  { PREFIX, NAME, HELPTEXT, METAVAR, OPT_##ID, Option::KIND##Class, PARAM, \
-FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS },
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, ARGVALUE)\
+  {PREFIX, NAME,  HELPTEXT,METAVAR, OPT_##ID,  Option::KIND##Class,\
+   PARAM,  FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS, ARGVALUE},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/tools/llvm-cvtres/llvm-cvtres.cpp
===
--- llvm/tools/llvm-cvtres/llvm-cvtres.cpp
+++ llvm/tools/llvm-cvtres/llvm-cvtres.cpp
@@ -37,7 +37,7 @@
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR)  \
+   HELPTEXT, METAVAR, ARGVALUE)\
   OPT_##ID,
 #include "Opts.inc"
 #undef OPTION
@@ -49,12 +49,12 @@
 
 static const opt::OptTable::Info InfoTable[] = {
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR)  \
+   HELPTEXT, METAVAR, ARGVALUE)\
   {\
-  PREFIX,  NAME, HELPTEXT, \
-  METAVAR, OPT_##ID, opt::Option::KIND##Class, \
-  PARAM,   FLAGS,OPT_##GROUP,  \
-  OPT_##ALIAS, ALIASARGS},
+  PREFIX,  NAME,  HELPTEXT,\
+  METAVAR, OPT_##ID,  opt::Option::KIND##Class,\
+  PARAM,   FLAGS, OPT_##GROUP, \
+  OPT_##ALIAS, ALIASARGS, ARGVALUE},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
===
--- llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
+++ llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
@@ -31,7 +31,7 @@
 
 enum {
   OPT_INVALID = 0,
-#define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11) OPT_##ID,
+#define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11, _12) OPT_##ID,
 #include "Options.inc"
 #undef OPTION
 };
@@ -41,11 +41,9 @@
 #undef PREFIX
 
 static const 

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

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi updated this revision to Diff 102961.
yamaguchi added a comment.

Made trivial change.


https://reviews.llvm.org/D33383

Files:
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/DriverOptions.cpp
  clang/test/Driver/autocomplete.c
  clang/utils/bash-autocomplete.sh
  lld/COFF/Driver.h
  lld/COFF/DriverUtils.cpp
  lld/ELF/Driver.h
  lld/ELF/DriverUtils.cpp
  lld/lib/Driver/DarwinLdDriver.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/Option/OptTable.cpp
  llvm/lib/Option/Option.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -196,6 +196,9 @@
   OS << ", nullptr";
 
 // The option meta-variable name (unused).
+OS << ", nullptr";
+
+// The option Values (unused for groups).
 OS << ", nullptr)\n";
   }
   OS << "\n";
@@ -285,6 +288,13 @@
 else
   OS << "nullptr";
 
+// The option Values. Used for shell autocompletion.
+OS << ", ";
+if (!isa(R.getValueInit("Values")))
+  write_cstring(OS, R.getValueAsString("Values"));
+else
+  OS << "nullptr";
+
 OS << ")\n";
   }
   OS << "#endif // OPTION\n";
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -18,8 +18,9 @@
 
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-   HELPTEXT, METAVAR) OPT_##ID,
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, ARGVALUE)\
+  OPT_##ID,
 #include "Opts.inc"
   LastOption
 #undef OPTION
@@ -36,10 +37,10 @@
 };
 
 static const OptTable::Info InfoTable[] = {
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-   HELPTEXT, METAVAR)   \
-  { PREFIX, NAME, HELPTEXT, METAVAR, OPT_##ID, Option::KIND##Class, PARAM, \
-FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS },
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, ARGVALUE)\
+  {PREFIX, NAME,  HELPTEXT,METAVAR, OPT_##ID,  Option::KIND##Class,\
+   PARAM,  FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS, ARGVALUE},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/tools/llvm-cvtres/llvm-cvtres.cpp
===
--- llvm/tools/llvm-cvtres/llvm-cvtres.cpp
+++ llvm/tools/llvm-cvtres/llvm-cvtres.cpp
@@ -37,7 +37,7 @@
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR)  \
+   HELPTEXT, METAVAR, ARGVALUE)\
   OPT_##ID,
 #include "Opts.inc"
 #undef OPTION
@@ -49,12 +49,12 @@
 
 static const opt::OptTable::Info InfoTable[] = {
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR)  \
+   HELPTEXT, METAVAR, ARGVALUE)\
   {\
-  PREFIX,  NAME, HELPTEXT, \
-  METAVAR, OPT_##ID, opt::Option::KIND##Class, \
-  PARAM,   FLAGS,OPT_##GROUP,  \
-  OPT_##ALIAS, ALIASARGS},
+  PREFIX,  NAME,  HELPTEXT,\
+  METAVAR, OPT_##ID,  opt::Option::KIND##Class,\
+  PARAM,   FLAGS, OPT_##GROUP, \
+  OPT_##ALIAS, ALIASARGS, ARGVALUE},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
===
--- llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
+++ llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
@@ -31,7 +31,7 @@
 
 enum {
   OPT_INVALID = 0,
-#define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11) OPT_##ID,
+#define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11, _12) OPT_##ID,
 #include "Options.inc"
 #undef OPTION
 };
@@ -41,11 +41,9 @@
 #undef PREFIX
 
 static const llvm::opt::OptTable

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

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi updated this revision to Diff 102960.
yamaguchi added a comment.

Made trivial bug fix.


https://reviews.llvm.org/D33383

Files:
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/DriverOptions.cpp
  clang/test/Driver/autocomplete.c
  clang/utils/bash-autocomplete.sh
  lld/COFF/Driver.h
  lld/COFF/DriverUtils.cpp
  lld/ELF/Driver.h
  lld/ELF/DriverUtils.cpp
  lld/lib/Driver/DarwinLdDriver.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/Option/OptTable.cpp
  llvm/lib/Option/Option.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -196,6 +196,9 @@
   OS << ", nullptr";
 
 // The option meta-variable name (unused).
+OS << ", nullptr";
+
+// The option Values (unused for groups).
 OS << ", nullptr)\n";
   }
   OS << "\n";
@@ -285,6 +288,13 @@
 else
   OS << "nullptr";
 
+// The option Values. Used for shell autocompletion.
+OS << ", ";
+if (!isa(R.getValueInit("Values")))
+  write_cstring(OS, R.getValueAsString("Values"));
+else
+  OS << "nullptr";
+
 OS << ")\n";
   }
   OS << "#endif // OPTION\n";
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -18,8 +18,9 @@
 
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-   HELPTEXT, METAVAR) OPT_##ID,
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, ARGVALUE)\
+  OPT_##ID,
 #include "Opts.inc"
   LastOption
 #undef OPTION
@@ -36,10 +37,10 @@
 };
 
 static const OptTable::Info InfoTable[] = {
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-   HELPTEXT, METAVAR)   \
-  { PREFIX, NAME, HELPTEXT, METAVAR, OPT_##ID, Option::KIND##Class, PARAM, \
-FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS },
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, ARGVALUE)\
+  {PREFIX, NAME,  HELPTEXT,METAVAR, OPT_##ID,  Option::KIND##Class,\
+   PARAM,  FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS, ARGVALUE},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/tools/llvm-cvtres/llvm-cvtres.cpp
===
--- llvm/tools/llvm-cvtres/llvm-cvtres.cpp
+++ llvm/tools/llvm-cvtres/llvm-cvtres.cpp
@@ -37,7 +37,7 @@
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR)  \
+   HELPTEXT, METAVAR, ARGVALUE)\
   OPT_##ID,
 #include "Opts.inc"
 #undef OPTION
@@ -49,12 +49,12 @@
 
 static const opt::OptTable::Info InfoTable[] = {
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR)  \
+   HELPTEXT, METAVAR, ARGVALUE)\
   {\
-  PREFIX,  NAME, HELPTEXT, \
-  METAVAR, OPT_##ID, opt::Option::KIND##Class, \
-  PARAM,   FLAGS,OPT_##GROUP,  \
-  OPT_##ALIAS, ALIASARGS},
+  PREFIX,  NAME,  HELPTEXT,\
+  METAVAR, OPT_##ID,  opt::Option::KIND##Class,\
+  PARAM,   FLAGS, OPT_##GROUP, \
+  OPT_##ALIAS, ALIASARGS, ARGVALUE},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
===
--- llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
+++ llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
@@ -31,7 +31,7 @@
 
 enum {
   OPT_INVALID = 0,
-#define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11) OPT_##ID,
+#define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11, _12) OPT_##ID,
 #include "Options.inc"
 #undef OPTION
 };
@@ -41,11 +41,9 @@
 #undef PREFIX
 
 static const llvm::opt::OptTabl

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

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi updated this revision to Diff 102959.
yamaguchi marked an inline comment as done.
yamaguchi added a comment.

Made trivial change.


https://reviews.llvm.org/D33383

Files:
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/DriverOptions.cpp
  clang/test/Driver/autocomplete.c
  clang/utils/bash-autocomplete.sh
  lld/COFF/Driver.h
  lld/COFF/DriverUtils.cpp
  lld/ELF/Driver.h
  lld/ELF/DriverUtils.cpp
  lld/lib/Driver/DarwinLdDriver.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/Option/OptTable.cpp
  llvm/lib/Option/Option.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -196,6 +196,9 @@
   OS << ", nullptr";
 
 // The option meta-variable name (unused).
+OS << ", nullptr";
+
+// The option Values (unused for groups).
 OS << ", nullptr)\n";
   }
   OS << "\n";
@@ -285,6 +288,13 @@
 else
   OS << "nullptr";
 
+// The option Values. Used for shell autocompletion.
+OS << ", ";
+if (!isa(R.getValueInit("Values")))
+  write_cstring(OS, R.getValueAsString("Values"));
+else
+  OS << "nullptr";
+
 OS << ")\n";
   }
   OS << "#endif // OPTION\n";
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -18,8 +18,9 @@
 
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-   HELPTEXT, METAVAR) OPT_##ID,
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, ARGVALUE)\
+  OPT_##ID,
 #include "Opts.inc"
   LastOption
 #undef OPTION
@@ -36,10 +37,10 @@
 };
 
 static const OptTable::Info InfoTable[] = {
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-   HELPTEXT, METAVAR)   \
-  { PREFIX, NAME, HELPTEXT, METAVAR, OPT_##ID, Option::KIND##Class, PARAM, \
-FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS },
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, ARGVALUE)\
+  {PREFIX, NAME,  HELPTEXT,METAVAR, OPT_##ID,  Option::KIND##Class,\
+   PARAM,  FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS, ARGVALUE},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/tools/llvm-cvtres/llvm-cvtres.cpp
===
--- llvm/tools/llvm-cvtres/llvm-cvtres.cpp
+++ llvm/tools/llvm-cvtres/llvm-cvtres.cpp
@@ -37,7 +37,7 @@
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR)  \
+   HELPTEXT, METAVAR, ARGVALUE)\
   OPT_##ID,
 #include "Opts.inc"
 #undef OPTION
@@ -49,12 +49,12 @@
 
 static const opt::OptTable::Info InfoTable[] = {
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR)  \
+   HELPTEXT, METAVAR, ARGVALUE)\
   {\
-  PREFIX,  NAME, HELPTEXT, \
-  METAVAR, OPT_##ID, opt::Option::KIND##Class, \
-  PARAM,   FLAGS,OPT_##GROUP,  \
-  OPT_##ALIAS, ALIASARGS},
+  PREFIX,  NAME,  HELPTEXT,\
+  METAVAR, OPT_##ID,  opt::Option::KIND##Class,\
+  PARAM,   FLAGS, OPT_##GROUP, \
+  OPT_##ALIAS, ALIASARGS, ARGVALUE},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
===
--- llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
+++ llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
@@ -31,7 +31,7 @@
 
 enum {
   OPT_INVALID = 0,
-#define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11) OPT_##ID,
+#define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11, _12) OPT_##ID,
 #include "Options.inc"
 #undef OPTION
 };
@@ -41,11 +41,9 @@
 #unde

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

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi updated this revision to Diff 102958.
yamaguchi marked 10 inline comments as done.
yamaguchi added a comment.

Cleaned suggestValueCompletions and bash-autocomplete.sh, and added tests.

We have passed flags and values from bash to clang like `clang 
--autocomplete=l,=,-stdlib`, but I've changed this to pass `clang 
--autocomplte=-stdlib=,l` instead. This is more simple.

Added support and test for -meabi flag.
This flag is corner case, because it doesn't take `=` suffix like `-stdlib=`.

When `clang -stdlib=[tab]` is pushed, we would give possible values 
(libstd++,..), and when `clang -stdlib= [tab]` is pushed, we just give files 
under current directory.
And when `clang -meabi [tab]` is pushed, we give possible values (gnu,..).


https://reviews.llvm.org/D33383

Files:
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/DriverOptions.cpp
  clang/test/Driver/autocomplete.c
  clang/utils/bash-autocomplete.sh
  lld/COFF/Driver.h
  lld/COFF/DriverUtils.cpp
  lld/ELF/Driver.h
  lld/ELF/DriverUtils.cpp
  lld/lib/Driver/DarwinLdDriver.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/Option/OptTable.cpp
  llvm/lib/Option/Option.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -196,6 +196,9 @@
   OS << ", nullptr";
 
 // The option meta-variable name (unused).
+OS << ", nullptr";
+
+// The option Values (unused for groups).
 OS << ", nullptr)\n";
   }
   OS << "\n";
@@ -285,6 +288,13 @@
 else
   OS << "nullptr";
 
+// The option Values. Used for shell autocompletion.
+OS << ", ";
+if (!isa(R.getValueInit("Values")))
+  write_cstring(OS, R.getValueAsString("Values"));
+else
+  OS << "nullptr";
+
 OS << ")\n";
   }
   OS << "#endif // OPTION\n";
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -18,8 +18,9 @@
 
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-   HELPTEXT, METAVAR) OPT_##ID,
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, ARGVALUE)\
+  OPT_##ID,
 #include "Opts.inc"
   LastOption
 #undef OPTION
@@ -36,10 +37,10 @@
 };
 
 static const OptTable::Info InfoTable[] = {
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-   HELPTEXT, METAVAR)   \
-  { PREFIX, NAME, HELPTEXT, METAVAR, OPT_##ID, Option::KIND##Class, PARAM, \
-FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS },
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, ARGVALUE)\
+  {PREFIX, NAME,  HELPTEXT,METAVAR, OPT_##ID,  Option::KIND##Class,\
+   PARAM,  FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS, ARGVALUE},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/tools/llvm-cvtres/llvm-cvtres.cpp
===
--- llvm/tools/llvm-cvtres/llvm-cvtres.cpp
+++ llvm/tools/llvm-cvtres/llvm-cvtres.cpp
@@ -37,7 +37,7 @@
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR)  \
+   HELPTEXT, METAVAR, ARGVALUE)\
   OPT_##ID,
 #include "Opts.inc"
 #undef OPTION
@@ -49,12 +49,12 @@
 
 static const opt::OptTable::Info InfoTable[] = {
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR)  \
+   HELPTEXT, METAVAR, ARGVALUE)\
   {\
-  PREFIX,  NAME, HELPTEXT, \
-  METAVAR, OPT_##ID, opt::Option::KIND##Class, \
-  PARAM,   FLAGS,OPT_##GROUP,  \
-  OPT_##ALIAS, ALIASARGS},
+  PREFIX,  NAME,  HELPTEXT,\
+  METAVAR, OPT_##ID,  opt::Option::KIND##Class,\
+  PARAM,   FLAGS, OPT

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

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi updated this revision to Diff 102946.
yamaguchi marked 5 inline comments as done.
yamaguchi added a comment.

Update patch. Rui's comment about the bash part is very reasonable.


https://reviews.llvm.org/D33383

Files:
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/DriverOptions.cpp
  clang/test/Driver/autocomplete.c
  clang/utils/bash-autocomplete.sh
  lld/COFF/Driver.h
  lld/COFF/DriverUtils.cpp
  lld/ELF/Driver.h
  lld/ELF/DriverUtils.cpp
  lld/lib/Driver/DarwinLdDriver.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/Option/OptTable.cpp
  llvm/lib/Option/Option.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -196,6 +196,9 @@
   OS << ", nullptr";
 
 // The option meta-variable name (unused).
+OS << ", nullptr";
+
+// The option Values (unused for groups).
 OS << ", nullptr)\n";
   }
   OS << "\n";
@@ -285,6 +288,13 @@
 else
   OS << "nullptr";
 
+// The option Values. Used for shell autocompletion.
+OS << ", ";
+if (!isa(R.getValueInit("ArgValues")))
+  write_cstring(OS, R.getValueAsString("ArgValues"));
+else
+  OS << "nullptr";
+
 OS << ")\n";
   }
   OS << "#endif // OPTION\n";
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -18,8 +18,9 @@
 
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-   HELPTEXT, METAVAR) OPT_##ID,
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, ARGVALUE)\
+  OPT_##ID,
 #include "Opts.inc"
   LastOption
 #undef OPTION
@@ -36,10 +37,10 @@
 };
 
 static const OptTable::Info InfoTable[] = {
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-   HELPTEXT, METAVAR)   \
-  { PREFIX, NAME, HELPTEXT, METAVAR, OPT_##ID, Option::KIND##Class, PARAM, \
-FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS },
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, ARGVALUE)\
+  {PREFIX, NAME,  HELPTEXT,METAVAR, OPT_##ID,  Option::KIND##Class,\
+   PARAM,  FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS, ARGVALUE},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/tools/llvm-cvtres/llvm-cvtres.cpp
===
--- llvm/tools/llvm-cvtres/llvm-cvtres.cpp
+++ llvm/tools/llvm-cvtres/llvm-cvtres.cpp
@@ -37,7 +37,7 @@
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR)  \
+   HELPTEXT, METAVAR, ARGVALUE)\
   OPT_##ID,
 #include "Opts.inc"
 #undef OPTION
@@ -49,12 +49,12 @@
 
 static const opt::OptTable::Info InfoTable[] = {
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR)  \
+   HELPTEXT, METAVAR, ARGVALUE)\
   {\
-  PREFIX,  NAME, HELPTEXT, \
-  METAVAR, OPT_##ID, opt::Option::KIND##Class, \
-  PARAM,   FLAGS,OPT_##GROUP,  \
-  OPT_##ALIAS, ALIASARGS},
+  PREFIX,  NAME,  HELPTEXT,\
+  METAVAR, OPT_##ID,  opt::Option::KIND##Class,\
+  PARAM,   FLAGS, OPT_##GROUP, \
+  OPT_##ALIAS, ALIASARGS, ARGVALUE},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
===
--- llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
+++ llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
@@ -31,7 +31,7 @@
 
 enum {
   OPT_INVALID = 0,
-#define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11) OPT_##ID,
+#define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11, _12) OPT_##ID,
 #include "Opti

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

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added inline comments.



Comment at: clang/utils/bash-autocomplete.sh:10
+  do
+arg="$arg""${COMP_WORDS[$i]},"
+  done

ruiu wrote:
> nit: you don't need double double-quotes. Instead, `"$arg${COMP_WORDS[$i]}," 
> should just work.
On second thought, I think what you actually want to do is to concatenate the 
elements of the array with `,`, without adding `,` at end of the string. For 
example, if COMP_WORDS contains "a", "b" and "c", you want to create "a,b,c" 
instead of "a,b,c," or ",a,b,c".

There are surprisingly large number of different ways of doing this, but it 
seems this is one of the easiest ways.

  arg="$(IFS=,; echo "${COMP_WORDS[*]}")"

IFS is a special shell variable containing a word splitting words. By setting 
the variable, COMP_WORDS[*] is expanded to (for example) "a,b,c" instead of the 
default "a b c".


https://reviews.llvm.org/D33383



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

As to the order of Command variable contents, I think I'm not convinced. You 
can access the last element as `Command[Command.size() - 1]` (or maybe 
`Command.back()`), no? It is slightly awkward than `Command[0]`, but that's not 
horribly hard to handle, and passing arguments in the reverse order seems more 
counter-intuitive to me.




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:
> 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.
I think I feel fairly strongly prefer "Value" over "ArgValue". If this is a 
one-time name, I agree that we should use "ArgValue", but this is not the case. 
We are going to add bunch of "Values" to options that take arguments, and I 
expect that the verbose name will start feeling too verbose. So please switch 
back to "Values".



Comment at: clang/utils/bash-autocomplete.sh:7
 
-  flags=$( clang --autocomplete="$cur" )
-  if [[ "$flags" == "" || "$cur" == "" ]]; then
+  arg=""
+  for (( i = $cword; i >= 1; i-- )) ;

Don't you want to make it `local`?



Comment at: clang/utils/bash-autocomplete.sh:8
+  arg=""
+  for (( i = $cword; i >= 1; i-- )) ;
+  do

If you don't write `do` on the same line as `for`, you don't need to write `;` 
at end of a line. I.e. write it in either

  for ...
  do
 command...
  done

or

  for ...; do
command...
  done

This is because you need a newline between `for` and `do`, and `;` works as a 
newline.



Comment at: clang/utils/bash-autocomplete.sh:10
+  do
+arg="$arg""${COMP_WORDS[$i]},"
+  done

nit: you don't need double double-quotes. Instead, `"$arg${COMP_WORDS[$i]}," 
should just work.



Comment at: clang/utils/bash-autocomplete.sh:15-16
+  if [[ "$cur" == "=" ]]; then
+cur=""
+COMPREPLY=( $( compgen -W "$flags" -- "$cur") )
+  else if [[ "$flags" == "" || "$cur" == "" ]]; then

Alternatively, `COMPRELY=( $( compgen -W "$flags" -- "") )` should work.



Comment at: clang/utils/bash-autocomplete.sh:17
+COMPREPLY=( $( compgen -W "$flags" -- "$cur") )
+  else if [[ "$flags" == "" || "$cur" == "" ]]; then
 _filedir

Use `elif` so that you don't have to write two `fi`s at end. In a bash script, 
if ... else if is usually written as

  if ...; then
  elif ...; then
  elif ...; then
  fi

If you use `else if` instead of `elif`, and if you indent the code properly, 
it'll look like

  if ...; then
  else
if ...; then
fi
  fi

This is why you needed two `fi`s.


https://reviews.llvm.org/D33383



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi marked 2 inline comments as done.
yamaguchi added inline comments.



Comment at: llvm/lib/Option/OptTable.cpp:198
+  continue;
+std::string S = "-" + std::string(In.Name);
+std::string C = (Command[1] == "=") ? 

yamaguchi wrote:
> teemperor wrote:
> > Hmm, will this work with "--" flags? Also, same as above with naming `S` 
> > and `C`.
> @teemperor 
> Do you think we should add some sample value completions to flags start with 
> "--" (such as --target) and flags which don't contain "=" (such as -mllvm 
> ) ?
> This part of code is rather ad hoc right now for -stdlib.
I had add support for this in this update as well, thanks.


https://reviews.llvm.org/D33383



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi added inline comments.



Comment at: clang/test/Driver/autocomplete.c:7
 // NONE: foo
+// RUN: %clang --autocomplete=l,=,-stdlib | FileCheck %s -check-prefix=STDLIB
+// STDLIB: libc++ libstdc++

ruiu wrote:
> Why do you want to pass the arguments in the reverse order?
In the reverse order, current flag is always exists at first, so I thought it 
is more simple. (Eg. `clang -fsyntax-only -o foo -std=c` will be passed as 
`c,=,-std,foo,-i,-fsyntax-only`)


https://reviews.llvm.org/D33383



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi updated this revision to Diff 102944.
yamaguchi marked 3 inline comments as done.
yamaguchi added a comment.
Herald added a subscriber: hiraditya.

Update patch and add support for `clang -stdlib=[tab]` case.
In this case we expect to see all possible values.
(Eg. `libc++ libstdc++ platform` for `clang -stdlib=[tab]`).


https://reviews.llvm.org/D33383

Files:
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/DriverOptions.cpp
  clang/test/Driver/autocomplete.c
  clang/utils/bash-autocomplete.sh
  lld/COFF/Driver.h
  lld/COFF/DriverUtils.cpp
  lld/ELF/Driver.h
  lld/ELF/DriverUtils.cpp
  lld/lib/Driver/DarwinLdDriver.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/Option/OptTable.cpp
  llvm/lib/Option/Option.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -196,6 +196,9 @@
   OS << ", nullptr";
 
 // The option meta-variable name (unused).
+OS << ", nullptr";
+
+// The option Values (unused for groups).
 OS << ", nullptr)\n";
   }
   OS << "\n";
@@ -285,6 +288,13 @@
 else
   OS << "nullptr";
 
+// The option Values. Used for shell autocompletion.
+OS << ", ";
+if (!isa(R.getValueInit("ArgValues")))
+  write_cstring(OS, R.getValueAsString("ArgValues"));
+else
+  OS << "nullptr";
+
 OS << ")\n";
   }
   OS << "#endif // OPTION\n";
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -18,8 +18,9 @@
 
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-   HELPTEXT, METAVAR) OPT_##ID,
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, ARGVALUE)\
+  OPT_##ID,
 #include "Opts.inc"
   LastOption
 #undef OPTION
@@ -36,10 +37,10 @@
 };
 
 static const OptTable::Info InfoTable[] = {
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
-   HELPTEXT, METAVAR)   \
-  { PREFIX, NAME, HELPTEXT, METAVAR, OPT_##ID, Option::KIND##Class, PARAM, \
-FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS },
+#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
+   HELPTEXT, METAVAR, ARGVALUE)\
+  {PREFIX, NAME,  HELPTEXT,METAVAR, OPT_##ID,  Option::KIND##Class,\
+   PARAM,  FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS, ARGVALUE},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/tools/llvm-cvtres/llvm-cvtres.cpp
===
--- llvm/tools/llvm-cvtres/llvm-cvtres.cpp
+++ llvm/tools/llvm-cvtres/llvm-cvtres.cpp
@@ -37,7 +37,7 @@
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR)  \
+   HELPTEXT, METAVAR, ARGVALUE)\
   OPT_##ID,
 #include "Opts.inc"
 #undef OPTION
@@ -49,12 +49,12 @@
 
 static const opt::OptTable::Info InfoTable[] = {
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR)  \
+   HELPTEXT, METAVAR, ARGVALUE)\
   {\
-  PREFIX,  NAME, HELPTEXT, \
-  METAVAR, OPT_##ID, opt::Option::KIND##Class, \
-  PARAM,   FLAGS,OPT_##GROUP,  \
-  OPT_##ALIAS, ALIASARGS},
+  PREFIX,  NAME,  HELPTEXT,\
+  METAVAR, OPT_##ID,  opt::Option::KIND##Class,\
+  PARAM,   FLAGS, OPT_##GROUP, \
+  OPT_##ALIAS, ALIASARGS, ARGVALUE},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
===
--- llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
+++ llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
@@ -31,7 +31,7 @@
 
 enum {
   OPT_INVALID = 0,
-#define OPTION(_1, _

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

[Vassil Vassilev via Phabricator via cfe-commits]

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 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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Rui Ueyama via Phabricator via cfe-commits]

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 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 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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits