[PATCH] D41206: [llvm-cov] Multi-threaded implementation of prepareFileReports method.

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: tools/llvm-cov/CoverageSummaryInfo.h:181
 
+  FileCoverageSummary()
+      : Name(), RegionCoverage(), LineCoverage(), FunctionCoverage(),
----------------
Dor1s wrote:
> Dor1s wrote:
> > vsk wrote:
> > > I don't think we should make it possible to create file summaries with empty names. Was this introduced so the std::vector could default-initialize FileCoverageSummary? If so, would make_unique<FileCoverageSummary[]>(N) suffice as an alternative?
> > Yes :( I'm not sure how `make_unique` may help, e.g. if I do:
> > 
> > ```
> > auto FileReports = std::make_unique<FileCoverageSummary[]>(Files.size());
> > ```
> > 
> > It still needs a default constructor. Did you mean `llvm::make_unique` by any chance? It still looks for an empty constructor though
> Either way, I found another solution. Let me upload it real quick so you can take a look :)
Looks good.


================
Comment at: tools/llvm-cov/CoverageSummaryInfo.h:190
+  FileCoverageSummary &operator+=(const FileCoverageSummary &RHS) {
+    assert((RHS.Name.empty() || RHS.Name == LHS.Name) &&
+           "Addition of file coverage summary from different files.");
----------------
Dor1s wrote:
> vsk wrote:
> > If we stipulate that file summaries have valid names, we could avoid a bit of this complexity.
> Sure!  But we still need to verify that `RHS.Name == LHS.Name`, right?
Right, that can't hurt.


https://reviews.llvm.org/D41206