[PATCH] D18756: [Coverage] Avoid unnecessary copying of an std::vector.

[Justin Bogner via llvm-commits]

Igor Kudrin <ikudrin.dev at gmail.com> writes:
> ikudrin retitled this revision from "[Coverage] Avoid unnecessary
> copying of an array. NFC." to "[Coverage] Avoid unnecessary copying of
> an std::vector.".
> ikudrin updated this revision to Diff 52660.
> ikudrin added a comment.
>
> - Use NRVO instead of std::move.
> - Prevent unexpected using of SegmentBuilder by making method
> buildSegments() static.

LGTM.

>
> http://reviews.llvm.org/D18756
>
> Files:
>   llvm/trunk/lib/ProfileData/CoverageMapping.cpp
>
> Index: llvm/trunk/lib/ProfileData/CoverageMapping.cpp
> ===================================================================
> --- llvm/trunk/lib/ProfileData/CoverageMapping.cpp
> +++ llvm/trunk/lib/ProfileData/CoverageMapping.cpp
> @@ -272,9 +272,11 @@
>  };
>  
>  class SegmentBuilder {
> -  std::vector<CoverageSegment> Segments;
> +  std::vector<CoverageSegment> &Segments;
>    SmallVector<const CountedRegion *, 8> ActiveRegions;
>  
> +  SegmentBuilder(std::vector<CoverageSegment> &Segments) : Segments(Segments) {}
> +
>    /// Start a segment with no count specified.
>    void startSegment(unsigned Line, unsigned Col) {
>      DEBUG(dbgs() << "Top level segment at " << Line << ":" << Col << "\n");
> @@ -318,9 +320,7 @@
>        startSegment(Line, Col, false, *ActiveRegions.back());
>    }
>  
> -public:
> -  /// Build a list of CoverageSegments from a sorted list of Regions.
> -  std::vector<CoverageSegment> buildSegments(ArrayRef<CountedRegion> Regions) {
> +  void buildSegmentsImpl(ArrayRef<CountedRegion> Regions) {
>      const CountedRegion *PrevRegion = nullptr;
>      for (const auto &Region : Regions) {
>        // Pop any regions that end before this one starts.
> @@ -341,6 +341,15 @@
>      // Pop any regions that are left in the stack.
>      while (!ActiveRegions.empty())
>        popRegion();
> +  }
> +
> +public:
> +  /// Build a list of CoverageSegments from a sorted list of Regions.
> +  static std::vector<CoverageSegment>
> +  buildSegments(ArrayRef<CountedRegion> Regions) {
> +    std::vector<CoverageSegment> Segments;
> +    SegmentBuilder Builder(Segments);
> +    Builder.buildSegmentsImpl(Regions);
>      return Segments;
>    }
>  };
> @@ -426,7 +435,7 @@
>  
>    sortNestedRegions(Regions.begin(), Regions.end());
>    DEBUG(dbgs() << "Emitting segments for file: " << Filename << "\n");
> -  FileCoverage.Segments = SegmentBuilder().buildSegments(Regions);
> +  FileCoverage.Segments = SegmentBuilder::buildSegments(Regions);
>  
>    return FileCoverage;
>  }
> @@ -468,7 +477,7 @@
>  
>    sortNestedRegions(Regions.begin(), Regions.end());
>    DEBUG(dbgs() << "Emitting segments for function: " << Function.Name << "\n");
> -  FunctionCoverage.Segments = SegmentBuilder().buildSegments(Regions);
> +  FunctionCoverage.Segments = SegmentBuilder::buildSegments(Regions);
>  
>    return FunctionCoverage;
>  }
> @@ -488,7 +497,7 @@
>    sortNestedRegions(Regions.begin(), Regions.end());
>    DEBUG(dbgs() << "Emitting segments for expansion of file " << Expansion.FileID
>                 << "\n");
> -  ExpansionCoverage.Segments = SegmentBuilder().buildSegments(Regions);
> +  ExpansionCoverage.Segments = SegmentBuilder::buildSegments(Regions);
>  
>    return ExpansionCoverage;
>  }
>
>