[PATCH] D96783: [Driver] Support -gdwarf64 for assembly files

[Igor Kudrin via Phabricator via cfe-commits]

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


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3757-3771
+  if (DwarfFormatArg &&
+      DwarfFormatArg->getOption().matches(options::OPT_gdwarf64)) {
+    if (DwarfVersion < 3)
+      D.Diag(diag::err_drv_argument_only_allowed_with)
+          << DwarfFormatArg->getAsString(Args) << "DWARFv3 or greater";
+    else if (!T.isArch64Bit())
+      D.Diag(diag::err_drv_argument_only_allowed_with)
----------------
dblaikie wrote:
> Could simplify this with an early return:
> ```
> if (!DwarfFormatArg)
>   return;
> ...
> ```
> 
> So DwarfFormatArg isn't tested twice/reduces indentation/etc.
Thanks! I'll update that on commit.


================
Comment at: clang/tools/driver/cc1as_main.cpp:237
+  if (auto *DwarfFormatArg = Args.getLastArg(OPT_gdwarf64, OPT_gdwarf32))
+    Opts.Dwarf64 = DwarfFormatArg->getOption().matches(OPT_gdwarf64) ? 1 : 0;
   Opts.DwarfVersion = getLastArgIntValue(Args, OPT_dwarf_version_EQ, 2, Diags);
----------------
dblaikie wrote:
> No need for the ` ? 1 : 0`, that's what booleans convert to anyway. (unless there's prior art/other instances of this idiom in this file that this is consistent with)
Thanks! I'll apply the suggestion on commit, too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96783