[PATCH] Code coverage tool that uses the new coverage mapping format and clang's instrumentation based profiling data

[Duncan P. N. Exon Smith]

> On 2014-Aug-21, at 12:21, Alex Lorenz <arphaman at gmail.com> wrote:
> 
> Thanks for the suggestions, I have updated the patch and have some comments:
> 
>    This pattern shows up a few times. It might be nicer to make an RAII
>    object for it and write something like:
> 
>      {
>        ColoredOutputRAII C(ViewOpts.Colors, outs(), raw_ostream::CYAN);
>        outs() << Function.PrettyName << " from " << SourceFile << ":";
>      }
> 
>     WDYT?
> 
> I've implemented something similar to this in the updated patch, please take a look.
> 
>   I wonder if we really need both Regions and SortedRegions in this
>   structure. It looks like regions is just used to unique the input, and
>   then when we want to use it, we sort first.
> 
>   How many duplicate entries do we expect? Would it make sense to add
>   items directly to the vector, and then std::unique it?
> 
> std::unique isn't really suitable as we have to sum the execution counts somehow. The number of duplicate entries depends on the particular source code - the use of macros and templates will increase this.

`std::unique` is simple to write and customize.  IIUC, your flow is to
gather and coalesce the regions, then sort them (for profit?).  You can
change this to gather, then sort and coalesce.  `std::unordered_map` is
a pretty heavy tool unless you actually need its iterator guarantees.

This is how you can sort and coalesce:

    if (Regions.size() <= 1)
      return;
    std::sort(Regions.begin(), Regions.end());
    auto O = Regions.begin();
    for (auto I = O + 1, E = Regions.end(); I != E; ++I)
      if (I->first == O->first)
        O->second += I->second;
      else
        *++O = std::move(*I);
    Regions.erase(++O, Regions.end());

>    Any reason this can't be a DenseMap?
> 
> It's simpler to use unordered_map here as DenseMap needs DenseMapInfo.

You've done most of the work already by writing a hash function.
`unordered_map<>` is a heavy tool, so you should prefer `DenseMap<>`
unless you need its guarantees.

> 
> I've also removed the demangling support as it's something that can be committed in a later patch.
> 
> http://reviews.llvm.org/D4445
> 
> Files:
>  tools/llvm-cov/CMakeLists.txt
>  tools/llvm-cov/CodeCoverage.cpp
>  tools/llvm-cov/CoverageFilters.cpp
>  tools/llvm-cov/CoverageFilters.h
>  tools/llvm-cov/CoverageReport.cpp
>  tools/llvm-cov/CoverageReport.h
>  tools/llvm-cov/CoverageSummary.cpp
>  tools/llvm-cov/CoverageSummary.h
>  tools/llvm-cov/CoverageSummaryInfo.cpp
>  tools/llvm-cov/CoverageSummaryInfo.h
>  tools/llvm-cov/CoverageViewOptions.h
>  tools/llvm-cov/FunctionCoverageMapping.h
>  tools/llvm-cov/LLVMBuild.txt
>  tools/llvm-cov/Makefile
>  tools/llvm-cov/RenderingSupport.h
>  tools/llvm-cov/SourceCoverageDataManager.cpp
>  tools/llvm-cov/SourceCoverageDataManager.h
>  tools/llvm-cov/SourceCoverageView.cpp
>  tools/llvm-cov/SourceCoverageView.h
>  tools/llvm-cov/llvm-cov.cpp
> <D4445.12790.patch>