[PATCH] D140263: [NFC] Vastly simplifies TypeSize

Guillaume Chatelet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 19 06:07:49 PST 2022


gchatelet marked an inline comment as done.
gchatelet added inline comments.


================
Comment at: llvm/include/llvm/Support/TypeSize.h:168
 
-  /// Add \p RHS to the value at the univariate dimension.
-  LeafTy getWithIncrement(ScalarTy RHS) const {
-    return static_cast<LeafTy>(
-        UnivariateLinearPolyBase(Value + RHS, UnivariateDim));
+  /// Add \p Rhs to the value at the univariate dimension.
+  LeafTy getWithIncrement(ScalarTy Rhs) const {
----------------
gchatelet wrote:
> barannikov88 wrote:
> > gchatelet wrote:
> > > barannikov88 wrote:
> > > > univariate dimension --> fixed quantity?
> > > > 
> > > Reworded it MinValue since this is what it is.
> > Thanks. However, MinValue is an implementation detail that should not be exposed even in comments.
> > Thanks. However, MinValue is an implementation detail that should not be exposed even in comments.
> 
> Is that true ? I'm under the impression that this is what it was intended for in the original patch
> https://github.com/llvm/llvm-project/commit/b302561b763a1d2eb1a450e135b8d49931936755
> 
> ```
> * Adds a TypeSize struct to represent the known minimum size of a type
>   along with a flag to indicate that the runtime size is a integer multiple
>   of that size
> ```
Let me rephrase "yes you're right that this is an implementation detail and it shouldn't be exposed but in that particular case it seems to be the exact semantic for the type".
I think it's better to update the documentation to reflect what was in the original comment. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140263



More information about the llvm-commits mailing list