[PATCH] D49366: Add support for /FUNCTIONPADMIN command-line option
Zachary Turner via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 16 09:31:58 PDT 2018
zturner added inline comments.
================
Comment at: COFF/PDB.cpp:118-122
+ Expected<const CVIndexMap &> mergeDebugT(ObjFile *File,
+ CVIndexMap &ObjectIndexMap);
- Expected<const CVIndexMap&> maybeMergeTypeServerPDB(ObjFile *File,
- TypeServer2Record &TS);
+ Expected<const CVIndexMap &> maybeMergeTypeServerPDB(ObjFile *File,
+ TypeServer2Record &TS);
----------------
stefan_reinalter wrote:
> zturner wrote:
> > It looks like you may have run clang-format against the entire source files, rather than against only the lines that you changed. Would it be possible to revert these changes and only clang-format changed lines? There's a script called git-clang-format in the clang source tree. Usually what we do is add this location to `PATH`, and then run `git clang-format` That will only format touched lines.
> Yes, I ran clang-format against all touched files. I thought this was the intention to make everything use the same format, but can revert the changes of course.
Usually we try to keep CLs limited to only changes that contribute to the functionality you're implementing in this patch. clang-formatting an entire file might be worthwile, but we should do it in a separate patch (before or after this patch doesn't matter) that way the only thing we see on the review is changes specifically related to implementing `/FUNCTIONPADMIN`
Repository:
rLLD LLVM Linker
https://reviews.llvm.org/D49366
More information about the llvm-commits
mailing list