dblaikie added a comment.

Thanks! - though on reflection I'm going to invoke @echristo again about the 
naming. It's unfortunately a bit backwards that the pre-standard flag is 
-gsplit-dwarf and what we're proposing as the standard flag is -fdwarf-fission, 
when the DWARF standard doesn't use the word "Fission" at all (only "split 
DWARF").

Maybe it'd be easy enough/better to add the "=single" suffix to the existing 
"-gsplit-dwarf"? (as much as I'm still a bit confused by which debug options 
use a '-g' and which ones use a '-f', etc).

Really sorry to go back/forth/around about on this, George.



================
Comment at: lib/Driver/ToolChains/Clang.cpp:2999-3001
+    StringRef Value = A->getOption().matches(options::OPT_fdwarf_fission_EQ)
+                          ? A->getValue()
+                          : "split";
----------------
Rather than creating a string in the case where there's no need for string 
matching/parsing, perhaps that could be avoided?

  if (!A->getOption().matches(options::OPT_fdwarf_fission_EQ))
    return DwarfFissionKind::Split;

  StringRef Value = A->getValue();
  if (Value == "split")
  ...




================
Comment at: lib/Driver/ToolChains/Clang.cpp:3010-3011
+  }
+  if (ArgIndex)
+    *ArgIndex = 0;
+  return DwarfFissionKind::None;
----------------
I'd probably either accept this as a precondition (assert(!ArgIndex || ArgIndex 
== 0)) or do this bit at the start of the function rather than the end here, to 
make it simpler/clearer that all paths assign some value to ArgIndex before 
exit of the function (even though that does mean checking/assigning ArgIndex 
unnecessarily in the case where the argument is given, etc)


================
Comment at: lib/Driver/ToolChains/Clang.cpp:5889
   const llvm::Triple &T = getToolChain().getTriple();
-  if (Args.hasArg(options::OPT_gsplit_dwarf) &&
+  if ((getDebugFissionKind(D, Args) == DwarfFissionKind::Split) &&
       (T.isOSLinux() || T.isOSFuchsia())) {
----------------
Could having more than one call to getDebugFissionKind lead to the error 
message (for -fdwarf-fission=garbage) being printed multiple times? Or is this 
call on a distinct/mutually exclusive codepath to the other?


https://reviews.llvm.org/D52296



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

Reply via email to