[PATCH] D141134: [NFC] Only expose getXXXSize functions in TypeSize

[Clement Courbet via Phabricator via llvm-commits]

courbet added a comment.

I think we should keep a single one, but just to bikeshed, a `TypeSize` has a `Value`, not a `Size`, so I would keep `getFixedValue`.



================
Comment at: llvm/include/llvm/Support/TypeSize.h:319-322
+  // Make 'getKnownMinValue' / 'getFixedValue' private as they are exposed as
+  // 'getKnownMinValue' / 'getFixedValue' lower in this class.
+  constexpr ScalarTy getKnownMinValue() const { return UP::getKnownMinValue(); }
+  constexpr ScalarTy getFixedValue() const { return UP::getFixedValue(); }
----------------
gchatelet wrote:
> sdesmalen wrote:
> > gchatelet wrote:
> > > sdesmalen wrote:
> > > > They're both only used once below, so I think you can remove the private interface and just call the parent class' method directly?
> > > I could but then the methods would still be visible. Another option is to mark the functions protected in `FixedOrScalableQuantity` and make them public only in `ElementCount`.
> > Ah of course, I didn't realise they were inherited. In that case I'm happy with this approach, because for TypeSize we made a stylistic exception to have something called `getKnownMinSize`, whereas in other circumstances the default `getKnownMinValue` is appropriate.
> I forgot that I could use `using` to make the functions private. It's better than redefining them.
Well that does not really make it private because you can cast to base and call. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141134