[PATCH] D115380: ADT: Make SmallVector::set_size() private

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

In D115380#3180829 <https://reviews.llvm.org/D115380#3180829>, @dexonsmith wrote:

> Re-uploading diff to trigger bots now that dependent patches are live.
>
> I'm measuring compile-time perf at http://llvm-compile-time-tracker.com for the full patch set to confirm this doesn't regress anything. I'll report back once I see results.

Results here:
http://llvm-compile-time-tracker.com/compare.php?from=36e7b8dd564b383d9b4ca38674dbed6337e642b3&to=d1af2835b46f1a84c28942dae10cb1761ad3a57b&stat=instructions
I'm happy with what's there so I'll continue pushing these in.



================
Comment at: llvm/include/llvm/ADT/SmallVector.h:587-589
+  // Hide set_size() to avoid misuse.
+  void set_size(size_type N) { SuperClass::set_size(N); }
+
----------------
dblaikie wrote:
> dexonsmith wrote:
> > dblaikie wrote:
> > > I think maybe this can be removed entirely - all the call sites use "this->set_size", so I think they'd find that in the base class?
> > My goal was to downgrade from `protected` to `private`, since there's no reason to let subclasses of SmallVectorImpl access this either. WDYT? Not worth it? Better comment?
> ah - somewhere between "not worth it" and "if you like".
> 
> If you want to do this, though - maybe a "using SuperClass::set_size" would be workable/better than the wrapper function? (not 100% sure that will work, but I think it should)
Yeah; I considered that but didn't try it since I also wasn't 100% sure it would work (and if it didn't I might not have found the unnecessary uses in SmallString). But it's cleaner -- don't even need a comment I think -- so I'll do it now.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115380/new/

https://reviews.llvm.org/D115380