[PATCH] D93401: [flang][driver] Add support for `-D`, `-U`

Andrzej Warzynski via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 4 08:03:13 PST 2021


awarzynski accepted this revision.
awarzynski added a comment.
This revision is now accepted and ready to land.

Thank you for working on this @FarisRehman , LGTM! From what I can tell you also addressed the point that @tskeith raised (updated commit msg and added a dedicated test). Ccould you wait another day or two before merging? Just in case people have some other comments/suggestions.



================
Comment at: clang/lib/Driver/ToolChains/Flang.h:27-31
+  /// Add the specified preprocessing options from Args to CmdArgs,
+  /// claiming the arguments used.
+  ///
+  /// \param Args The list of input arguments
+  /// \param CmdArgs The list of output arguments
----------------
awarzynski wrote:
> 1. Could you clarify the difference between `Args` and `CmdArgs`?
> 2. Could you use doxygen's syntax to specify the parameter _direction_? E.g.:
> ```
>   /// \param [in] Args The list of input arguments
>   /// \param [out] CmdArgs The list of output arguments
> ```
> See https://www.doxygen.nl/manual/commands.html#cmdparam.
[nit] One minor suggestion:
```
  /// \param Args The list of input driver arguments
  /// \param CmdArgs The list of output command arguments
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93401



More information about the cfe-commits mailing list