cmtice marked 3 inline comments as done.
cmtice added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3246-3247
DWARFVersion = ExplicitVersion;
}
+ else if (DefaultDWARFVersion != 0)
+ DWARFVersion = DefaultDWARFVersion;
----------------
dblaikie wrote:
> dblaikie wrote:
> > 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))
> Oh, there's also clang/tools/clang-format/git-clang-format for formatting
> anything in a git revision range.
Ok, will do.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3247-3248
}
+ else if (DefaultDWARFVersion != 0)
+ DWARFVersion = DefaultDWARFVersion;
----------------
dblaikie wrote:
> 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).
This covers the case where you are setting the default dwarf version without
actually turning on/off debug info at all (there's no -gdwarf or -g option, so
EmitDwarf is false).
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6173-6175
+ unsigned DefaultDwarfVersion = ParseDebugDefaultVersion(getToolChain(),
Args);
+ DwarfVersion = DefaultDwarfVersion ? DefaultDwarfVersion
+ :getToolChain().GetDefaultDwarfVersion();
----------------
dblaikie wrote:
> 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?
Ok will do.
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
