[PATCH] D113250: [clang][driver] Add -fplugin-arg- to pass arguments to plugins

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:150
+      if (args[0] == "help") {
+        llvm::errs() << "Help for the CallSuperAttr plugin goes here\n";
+      }
----------------
https://llvm.org/docs/CodingStandards.html#do-not-use-braced-initializer-lists-to-call-a-constructor


================
Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:193
+def warn_drv_missing_plugin_name : Warning<
+  "Missing plugin name in %0">;
+def warn_drv_missing_plugin_arg : Warning<
----------------
don't capitalize messages. See other diagnostics and https://llvm.org/docs/CodingStandards.html#error-and-warning-messages


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6678
+    A->claim();
+    if (FirstDashIndex == StringRef::npos || Arg == "") {
+      if (PluginName == "") {
----------------
empty()


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6679
+    if (FirstDashIndex == StringRef::npos || Arg == "") {
+      if (PluginName == "") {
+        D.Diag(diag::warn_drv_missing_plugin_name) << A->getAsString(Args);
----------------



================
Comment at: clang/test/Driver/plugin-driver-args.cpp:1
+// Test passing args to plugins via the clang driver and -fplugin-arg
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -fplugin-arg-call_super_plugin-help -fsyntax-only -### %s 2>&1 | FileCheck %s
----------------
I usually use `///` for non-RUN non-CHECK lines to make comments stand out (in some editors the highlight will even be different). That is a rule in some directories (llvm/test/tools lld/test) but not so consistent in other directories.


================
Comment at: clang/test/Driver/plugin-driver-args.cpp:11
+// Dashes cannot be part of the plugin name here
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -fplugin-arg-call_super_plugin-help-long -fsyntax-only %s 2>&1 -### | FileCheck %s --check-prefix=CHECK-CMD
+// CHECK-CMD: "-plugin-arg-call_super_plugin" "help-long"
----------------
I assume that you have checked `// REQUIRES: plugins, examples` is not needed, i.e. the test still passes if -DLLVM_ENABLE_PLUGINS=off


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113250/new/

https://reviews.llvm.org/D113250