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

[tbaeder]

Summary

From GCC's manpage:
-fplugin-arg-name-key=value

Define an argument called key with a value of value for the
plugin called name.

Since we don't have a key-value pair similar to gcc's plugin_argument
struct, simply accept key=value here anyway and pass it along as-is to
plugins.

This translates to the already existing '-plugin-arg-pluginname arg'
that clang cc1 accepts.

There is an ambiguity here because in clang, both the plugin name
as well as the option name can contain dashes, so when e.g. passing

-fplugin-arg-foo-bar-foo

it is not clear whether the plugin is foo-bar and the option is foo,
or the plugin is foo and the option is bar-foo. GCC solves this by
interpreting all dashes as part of the option name. So dashes can't be
part of the plugin name in this case.

[MaskRay]

The idea LGTM. You need a test in test/Driver checking how the driver option maps to the CC1 option.

You may check whether REQUIRES: plugins, examples is needed.

clang/lib/Driver/ToolChains/Clang.cpp

6672

What if FirstDashIndex == std::string::npos?

[MaskRay]

Then please also update docs/ClangPlugins.rst to use driver option instead of CC1 -plugin-arg

[tbaeder]

I already had a test but forgot to add the file to my local git commit, sorry.

[tbaeder]

clang/lib/Driver/ToolChains/Clang.cpp

6672

Things still "work", but the plugin name then gets parsed incorrectly. I've decided to skip this case now.

[MaskRay]

clang/docs/ClangPlugins.rst

133

Mention whether <argument> can contain -

clang/test/Frontend/plugin-driver-args.cpp

2 ↗

(On Diff #388129)

Move to test/Driver and use use -### to test CC1 options.

Check whether REQUIRES: plugins, examples is really needed. Many driver options don't need frontend feature availability.

Change plugin-call-super.cpp to use %clang_cc1 to test CC1 options.

[MaskRay]

clang/docs/ClangPlugins.rst

131

141

clang/lib/Driver/ToolChains/Clang.cpp

6675

This probably should be an error. See elsewhere in the file for error reporting.

6678

clang/test/Driver/plugin-driver-args.cpp

6

CHECK-SAME

[tbaeder]

Added two new warning types and used them for diagnosing malformed -fplugin-arg options. Also added tests for that.

[MaskRay]

clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp

150

https://llvm.org/docs/CodingStandards.html#do-not-use-braced-initializer-lists-to-call-a-constructor

clang/include/clang/Basic/DiagnosticDriverKinds.td

193

don't capitalize messages. See other diagnostics and https://llvm.org/docs/CodingStandards.html#error-and-warning-messages

clang/lib/Driver/ToolChains/Clang.cpp

6677

empty()

6678

clang/test/Driver/plugin-driver-args.cpp

2

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.

12

I assume that you have checked // REQUIRES: plugins, examples is not needed, i.e. the test still passes if -DLLVM_ENABLE_PLUGINS=off

[tbaeder]

clang/test/Driver/plugin-driver-args.cpp

12

I did, but I didn't delete the local CallSuperAttr.so. After doing that, they failed. I hope using -### as well here is fine.

[tbaeder]

Addressed review comments and fixed the tests

[tbaeder]

Hey @MaskRay, I know you've already approved but just to be sure: does the last round of changes look good?

Thanks

[MaskRay]

Thanks!

clang/docs/ClangPlugins.rst

 example is checked into the clang repository; please take a look at
 the `latest version of PrintFunctionNames.cpp
 <https://github.com/llvm/llvm-project/blob/main/clang/examples/PrintFunctionNames/PrintFunctionNames.cpp>`_.
 
 Running the plugin
 ==================
 
 
+Using the compiler driver
+--------------------------
+
+The Clang driver accepts the `-fplugin` option to load a plugin.
+Clang plugins can receive arguments from the compiler driver command
+line via the `fplugin-arg-<plugin name>-<argument>` option. Using this
+method, the plugin name cannot contain dashes itself, but the argument
+passed to the plugin can.
+
+
+.. code-block:: console
+
+  $ export BD=/path/to/build/directory
+  $ make -C $BD CallSuperAttr
+  $ clang++ -fplugin=$BD/lib/CallSuperAttr.so \
+            -fplugin-arg-call_super_plugin-help \
+            test.cpp
+
+If your plugin name contains dashes, either rename the plugin or used the
+cc1 command line options listed below.
+
+
 Using the cc1 command line
 --------------------------
 
 To run a plugin, the dynamic library containing the plugin registry must be
 loaded via the `-load` command line option. This will load all plugins
 that are registered, and you can select the plugins to run by specifying the
 `-plugin` option. Additional parameters for the plugins can be passed with
 `-plugin-arg-<plugin-name>`.

clang/docs/ReleaseNotes.rst

 -------------------------------------------------
 
 - Maximum _ExtInt size was decreased from 16,777,215 bits to 8,388,608 bits.
   Motivation for this was discussed in PR51829.
 
 New Compiler Flags
 ------------------
 
-- ...
+- Clang plugin arguments can now be passed through the compiler driver via
+  ``-fplugin-arg-pluginname-arg``, similar to GCC's ``-fplugin-arg``.
 
 Deprecated Compiler Flags
 -------------------------
 
 - -Wweak-template-vtables has been deprecated and no longer has any effect. The
   flag will be removed in the next release.
 
 Modified Compiler Flags

clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp

 public:
   std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
                                                  llvm::StringRef) override {
     return std::make_unique<CallSuperConsumer>();
   }
 
   bool ParseArgs(const CompilerInstance &CI,
                  const std::vector<std::string> &args) override {
+    if (!args.empty() && args[0] == "help")
+      llvm::errs() << "Help for the CallSuperAttr plugin goes here\n";
     return true;
   }
 
   PluginASTAction::ActionType getActionType() override {
     return AddBeforeMainAction;
   }
 };
 
 struct CallSuperAttrInfo : public ParsedAttrInfo {

clang/include/clang/Basic/DiagnosticDriverKinds.td

 def err_drv_force_crash : Error<
   "failing because %select{environment variable 'FORCE_CLANG_DIAGNOSTICS_CRASH' is set|'-gen-reproducer' is used}0">;
 def err_drv_invalid_mfloat_abi : Error<
   "invalid float ABI '%0'">;
 def err_drv_invalid_mtp : Error<
   "invalid thread pointer reading mode '%0'">;
 def err_drv_missing_arg_mtp : Error<
   "missing argument to '%0'">;
+def warn_drv_missing_plugin_name : Warning<
+  "missing plugin name in %0">,
+  InGroup<InvalidCommandLineArgument>;
+def warn_drv_missing_plugin_arg : Warning<
+  "missing plugin argument for plugin %0 in %1">,
+  InGroup<InvalidCommandLineArgument>;
 def err_drv_invalid_libcxx_deployment : Error<
   "invalid deployment target for -stdlib=libc++ (requires %0 or later)">;
 def err_drv_invalid_argument_to_option : Error<
   "invalid argument '%0' to -%1">;
 def err_drv_malformed_sanitizer_ignorelist : Error<
   "malformed sanitizer ignorelist: '%0'">;
 def err_drv_malformed_sanitizer_coverage_allowlist : Error<
   "malformed sanitizer coverage allowlist: '%0'">;

clang/include/clang/Driver/Options.td

defm ropi : BoolFOption<"ropi",
   PosFlag<SetTrue, [CC1Option], "Generate read-only position independent code (ARM only)">,
   NegFlag<SetFalse>>;
 defm rwpi : BoolFOption<"rwpi",
   LangOpts<"RWPI">, DefaultFalse,
   PosFlag<SetTrue, [CC1Option], "Generate read-write position independent code (ARM only)">,
   NegFlag<SetFalse>>;
 def fplugin_EQ : Joined<["-"], "fplugin=">, Group<f_Group>, Flags<[NoXarchOption]>, MetaVarName<"<dsopath>">,
   HelpText<"Load the named plugin (dynamic shared object)">;
+def fplugin_arg : Joined<["-"], "fplugin-arg-">,
+  MetaVarName<"<name>-<arg>">,
+  HelpText<"Pass <arg> to plugin <name>">;
 def fpass_plugin_EQ : Joined<["-"], "fpass-plugin=">,
   Group<f_Group>, Flags<[CC1Option]>, MetaVarName<"<dsopath>">,
   HelpText<"Load pass plugin from a dynamic shared object file (only with new pass manager).">,
   MarshallingInfoStringVector<CodeGenOpts<"PassPlugins">>;
 defm preserve_as_comments : BoolFOption<"preserve-as-comments",
   CodeGenOpts<"PreserveAsmComments">, DefaultTrue,
   NegFlag<SetFalse, [CC1Option], "Do not preserve comments in inline assembly">,
   PosFlag<SetTrue>>;

clang/lib/Driver/ToolChains/Clang.cpp

 
   // Turn -fplugin=name.so into -load name.so
   for (const Arg *A : Args.filtered(options::OPT_fplugin_EQ)) {
     CmdArgs.push_back("-load");
     CmdArgs.push_back(A->getValue());
     A->claim();
   }
 
+  // Turn -fplugin-arg-pluginname-key=value into
+  // -plugin-arg-pluginname key=value
+  // GCC has an actual plugin_argument struct with key/value pairs that it
+  // passes to its plugins, but we don't, so just pass it on as-is.
+  //
+  // The syntax for -fplugin-arg- is ambiguous if both plugin name and
+  // argument key are allowed to contain dashes. GCC therefore only
+  // allows dashes in the key. We do the same.
+  for (const Arg *A : Args.filtered(options::OPT_fplugin_arg)) {
+    auto ArgValue = StringRef(A->getValue());
+    auto FirstDashIndex = ArgValue.find('-');
+    StringRef PluginName = ArgValue.substr(0, FirstDashIndex);
+    StringRef Arg = ArgValue.substr(FirstDashIndex + 1);
+
+    A->claim();
+    if (FirstDashIndex == StringRef::npos || Arg.empty()) {
+      if (PluginName.empty()) {
+        D.Diag(diag::warn_drv_missing_plugin_name) << A->getAsString(Args);
+      } else {
+        D.Diag(diag::warn_drv_missing_plugin_arg)
+            << PluginName << A->getAsString(Args);
+      }
+      continue;
+    }
+
+    CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-arg-") + PluginName));
+    CmdArgs.push_back(Args.MakeArgString(Arg));
+  }
+
   // Forward -fpass-plugin=name.so to -cc1.
   for (const Arg *A : Args.filtered(options::OPT_fpass_plugin_EQ)) {
     CmdArgs.push_back(
         Args.MakeArgString(Twine("-fpass-plugin=") + A->getValue()));
     A->claim();
   }
 
   // Setup statistics file output.

clang/test/Driver/plugin-driver-args.cpp

+/// 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
+
+// CHECK: "-load"
+// CHECK-SAME: CallSuperAttr
+// CHECK-SAME: "-plugin-arg-call_super_plugin"
+// CHECK-SAME: "help"
+
+/// Check that dashed-args get forwarded like this to the plugin.
+/// 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"
+
+/// Error handling for -fplugin-arg-
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -fplugin-arg- -fsyntax-only %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-PLUGIN-NAME
+// CHECK-NO-PLUGIN-NAME: missing plugin name in -fplugin-arg-
+
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -fplugin-arg-testname -fsyntax-only %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-PLUGIN-ARG1
+// CHECK-NO-PLUGIN-ARG1: missing plugin argument for plugin testname in -fplugin-arg-testname
+
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -fplugin-arg-testname- -fsyntax-only %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-PLUGIN-ARG2
+// CHECK-NO-PLUGIN-ARG2: missing plugin argument for plugin testname in -fplugin-arg-testname-

clang/test/Frontend/plugin-call-super.cpp

-// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -fsyntax-only -Xclang -verify=callsuper %s
-// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -DBAD_CALLSUPER -fsyntax-only -Xclang -verify=badcallsuper %s
+// RUN: %clang_cc1 -load %llvmshlibdir/CallSuperAttr%pluginext -fsyntax-only -verify=callsuper %s
+// RUN: %clang_cc1 -load %llvmshlibdir/CallSuperAttr%pluginext -DBAD_CALLSUPER -fsyntax-only -verify=badcallsuper %s
 // REQUIRES: plugins, examples
 
 // callsuper-no-diagnostics
 struct Base1 {
   [[clang::call_super]] virtual void Test() {}
 };
 struct Base2 {
   [[clang::call_super]] virtual void Test() {}