vsk added a comment. In https://reviews.llvm.org/D25040#556209, @arphaman wrote:
> In https://reviews.llvm.org/D25040#556165, @vsk wrote: > > > In https://reviews.llvm.org/D25040#556109, @arphaman wrote: > > > > > Thanks for working on this, I have a couple of comments/questions: > > > > > > - The current logic doesn't work correctly for a test case that's shown > > > below: > > > > > > ``` class C2 { void __attribute__((unused)) m4() { } void > > > __attribute__((unused)) m5(); }; ``` The method 'm4' actually gets > > > covered with a zero region. I suspect you might have to perform the check > > > for the attribute in the 'EmptyCoverageMappingBuilder' class as well. > > > > > > Thanks, I'll take a look at that. > > > > > - I understand the value in ignoring coverage for unused functions which > > > aren't actually used anywhere. But people can still use them freely, so > > > do you think that it would be a good idea to provide coverage for those > > > functions when they are actually used? > > > > > > Yeah. Things like readCpuInfo() (lib/Support/Host.cpp) are marked 'unused', > > but we shouldn't suppress coverage for them. > > > > Based on a quick skim [1], it seems like most functions marked 'unused' > > aren't called normally. Maybe that's all the more reason to provide > > coverage for them: if they *are* executed, it would be helpful to know. But > > the common case seems to be 'dump' or 'verify' methods that are called from > > lldb/gtest, and I don't think it's useful to have coverage for these. > > > > I'm not sure how we'd determine that the function is actually used. Do you > > have an approach in mind? IIRC libclang has an API which allows you to > > count the users of a USR, but this isn't helpful without a view of all > > translation units. > > > We could create a special 'unused' region type which would cover unused > functions. If a function is marked as unused, its first region will be the > special one, and the normal regions will follow the special one. llvm-cov > would then be able to ignore coverage for functions with the 'unused' first > region if the function counter is zero, but it will still show it when unused > functions have been actually executed. In terms of serialization the unused > regions can be handled similarly to the 'SkippedRegion' type, so they won't > increase the size of coverage mapping for functions that don't use the unused > attribute. I think this is the best approach, but afaict it would require updating the coverage mapping format (http://llvm.org/docs/CoverageMappingFormat.html#pseudo-counter). That seems like a lot of work for not very much gain.. I might sit on this patch for a while until/unless I can think of something simpler. https://reviews.llvm.org/D25040 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
