[PATCH] Initial code coverage mapping data, reader, writer + C interface for ProfileData

[Bob Wilson]

> On Jul 18, 2014, at 5:34 PM, Alex L <arphaman at gmail.com> wrote:
> 
> Thanks.
> I was also wondering using a namespace for the coverage structures and readers/writers - I feel like taking over the name Counter in the llvm's namespace for coverage might not be such a good idea.

Sure, a separate namespace would make sense

> 
> 
> 2014-07-18 17:13 GMT-07:00 Bob Wilson <bob.wilson at apple.com>:
> 
>> On Jul 18, 2014, at 10:24 AM, Alex Lorenz <arphaman at gmail.com> wrote:
>> 
>> Thanks again for the review, I've implemented the suggested changes. I also merged the coverage mapping data into one section - this reduced the object file size for the test programs by 4.5% on average.
>> 
>> http://reviews.llvm.org/D4301
> 
> Great! I was just hoping to simplify things by having fewer sections, so reducing the size overhead is a nice bonus.
> 
> I have two other minor comments:
> 
> 1) I noticed when reviewing your clang patch that you didn't consistently follow the coding standard with regard to function names starting with a lowercase letter, and I see some instances of that here, too. For example, CounterExpressionBuilder has some lowercase function (get, getExpressions) but also some uppercase ones (ExtractTerms, Simplify, Add and Subtract). By my reading of the coding convention, these should all start with lowercase letters. I think there are other instances of that. Please check.
> 
> 2) I had suggested that you define a constant for Counter::EncodingTagBits + 1, which you did. But, you now have definitions of EncodingCounterTagAndExpansionRegionTagBits in both the reader and writer. I understand that this is only used internally, but it would be better to just define a single constant, probably alongside EncodingTagBits (i.e., in struct Counter).
> 
> Please commit with those changes (or if you don’t have commit access yet, I can do it for you.)
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140718/bf45161d/attachment.html>