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

[Justin Bogner]

Alex Lorenz <arphaman at gmail.com> writes:
> 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 like it, but I think it'd help readability if instead of:

    outs() << ostream_color(raw_ostream::RED) << ...

it was used like:

    colored_ostream(outs(), raw_ostream::RED) << ...

This makes it a bit more obvious that it lasts just for the expression.

>    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.
>
>     Any reason this can't be a DenseMap?
>
> It's simpler to use unordered_map here as DenseMap needs DenseMapInfo.

Sure.

> I've also removed the demangling support as it's something that can be
> committed in a later patch.

Thanks!

This is looking pretty good now, but it really needs some tests to go
with it.