[flang-commits] [PATCH] D123237: [flang] Fix LBOUND() folding for constant arrays

[Mike K via Phabricator via flang-commits]

FruitClover added inline comments.


================
Comment at: flang/include/flang/Evaluate/tools.h:1044
+template <typename T>
+Constant<T> PackageConstant(
+    const ConstantSubscripts &&bounds, bool asScalar = false) {
----------------
jeanPerier wrote:
> I think `PackageConstant` is very specific to the LBOUND/UBOUND folding usage. I am not sure it is worth exposing it in this widely included header.
> 
> Looking at `GetConstantArrayLboundHelper`, it is only ever used in folding, so maybe `PackageConstant` could be moved inside it, or next to it if that brings too many templates.
Thanks a lot for details, moved the class closer. 


================
Comment at: flang/lib/Evaluate/fold-integer.cpp:40
     // Strip off the parentheses
     return GetLbound(x.left());
   }
----------------
jeanPerier wrote:
> Since you are fixing LBOUND and UBOUND bugs, I think this part of the previous code is incorrect, as far as I can tell, the lower bounds of `(cst)` are ones, not the ones of cst.
> 
> e.g:
> ```
>    integer, parameter :: a3(2:4,4:6) = 0
>    print *, LBOUND((a3), 1)
> end
> ```
> 
> Should print `1`, not `2`, and I think returning `GetLbound(x.left())` will cause it to return `2`.
Oh, that's an interesting case, submitted https://reviews.llvm.org/D123838 based on current PR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123237