dblaikie added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3246-3247
DWARFVersion = ExplicitVersion;
}
+ else if (DefaultDWARFVersion != 0)
+ DWARFVersion = DefaultDWARFVersion;
----------------
Looks like this should be on a single line to conform to LLVM convention
(though might just be phabricator doing something weird)
If you can run clang-format over the change (not over the whole file) it should
fix up issues like this. (there's various clang-format editor integrations -
there's some google-internal documentation at go/clang-format that'll explain
how to setup an auto-save hook that'll clang-format the changed lines so all
your C++ code in the LLVM repository conforms to LLVM's coding conventions
(well, those that can be expressed by clang-format))
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3247-3248
}
+ else if (DefaultDWARFVersion != 0)
+ DWARFVersion = DefaultDWARFVersion;
----------------
Hmm, actually - why is this case necessary/what does it cover? I was hoping the
case you added inside the "if (EmitDwarf)" case above would be all that was
required (& the call to ParseDebugDefaultVersion would go inside there at the
use, to reduce the variable scope/keep the code closer together).
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6173-6175
+ unsigned DefaultDwarfVersion = ParseDebugDefaultVersion(getToolChain(),
Args);
+ DwarfVersion = DefaultDwarfVersion ? DefaultDwarfVersion
+ :getToolChain().GetDefaultDwarfVersion();
----------------
Some clang-formatting required, but also could probably be written as:
if (DwarfVersion == 0)
DwarfVersion = ParseDebugDefaultVersion(...);
if (DwarfVersion == 0)
DwarfVersion = getToolChain().GetDefaultDwarfVersion();
To make these more symmetric?
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69822/new/
https://reviews.llvm.org/D69822
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
