[PATCH] D18213: Add a module Hash in the bitcode and the combined index.

[Teresa Johnson via llvm-commits]

On Tue, Mar 29, 2016 at 5:03 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

>
> > On 2016-Mar-29, at 17:01, Mehdi Amini <mehdi.amini at apple.com> wrote:
> >
> >
> >> On Mar 29, 2016, at 4:58 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> >>
> >>
> >>> On 2016-Mar-29, at 16:16, Mehdi AMINI <mehdi.amini at apple.com> wrote:
> >>>
> >>> joker.eph updated this revision to Diff 52002.
> >>> joker.eph marked 6 inline comments as done.
> >>> joker.eph added a comment.
> >>>
> >>> Taking comments into account.
> >>>
> >>>
> >>> http://reviews.llvm.org/D18213
> >>>
> >>> Files:
> >>> include/llvm/Bitcode/BitstreamReader.h
> >>> include/llvm/Bitcode/LLVMBitCodes.h
> >>> include/llvm/Bitcode/ReaderWriter.h
> >>> include/llvm/IR/ModuleSummaryIndex.h
> >>> lib/Bitcode/Reader/BitcodeReader.cpp
> >>> lib/Bitcode/Writer/BitcodeWriter.cpp
> >>> lib/IR/ModuleSummaryIndex.cpp
> >>> lib/Transforms/IPO/FunctionImport.cpp
> >>> tools/llvm-as/llvm-as.cpp
> >>> tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp
> >>>
> >>> <D18213.52002.patch>
> >>
> >> Found a weird spacing issue below (I haven't looked deeply; I figure
> >> Rafael and Teresa are already looking at this one?  Happy to look if
> >> need be though.)
> >>
> >>> Index: include/llvm/Bitcode/LLVMBitCodes.h
> >>> ===================================================================
> >>> --- include/llvm/Bitcode/LLVMBitCodes.h
> >>> +++ include/llvm/Bitcode/LLVMBitCodes.h
> >>> @@ -71,43 +71,46 @@
> >>> enum { BITCODE_CURRENT_EPOCH = 0 };
> >>>
> >>>  /// MODULE blocks have a number of optional fields and subblocks.
> >>> -  enum ModuleCodes {
> >>> -    MODULE_CODE_VERSION     = 1,    // VERSION:     [version#]
> >>> -    MODULE_CODE_TRIPLE      = 2,    // TRIPLE:      [strchr x N]
> >>> -    MODULE_CODE_DATALAYOUT  = 3,    // DATALAYOUT:  [strchr x N]
> >>> -    MODULE_CODE_ASM         = 4,    // ASM:         [strchr x N]
> >>> -    MODULE_CODE_SECTIONNAME = 5,    // SECTIONNAME: [strchr x N]
> >>> +enum ModuleCodes {
> >>> +  MODULE_CODE_VERSION = 1,     // VERSION:     [version#]
> >>> +  MODULE_CODE_TRIPLE = 2,      // TRIPLE:      [strchr x N]
> >>> +  MODULE_CODE_DATALAYOUT = 3,  // DATALAYOUT:  [strchr x N]
> >>> +  MODULE_CODE_ASM = 4,         // ASM:         [strchr x N]
> >>> +  MODULE_CODE_SECTIONNAME = 5, // SECTIONNAME: [strchr x N]
> >>
> >> This spacing change seems unrelated (and inconsistent with the enums
> >> above and below
> >
> > Oh, that's clang-format, my first iterations on the patch I took care to
> manually revert the formatting in this file, and then I gave up.
> > I can reduce the diff to the only line change on commit though.
>
> If the clang-formatted version is better or not much worse (not always
> true for large switch statements and enums), then you can do an NFC
> prep commit that clang-formats the entire file and rebase on top of
> that.  Saves everyone some hassle.


I always wondered about that - any time I have touched this file and then
run clang-format on my changes it tries to reformat the whole thing and I
have to remember to clang-format the individual files in my patch excluding
this one. If the clang-format version is ok that would be great to commit
it as an NFC change.

Thanks,
Teresa



-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160330/69726d2b/attachment.html>