[flang-commits] [PATCH] D144743: [flang] Fix lowering of optional char proc args

[Leandro Lupori via Phabricator via flang-commits]

luporl added a comment.

Thanks for the review!



================
Comment at: flang/lib/Lower/ConvertExpr.cpp:2474-2485
+        mlir::Type funcTy = argTy.cast<mlir::TupleType>()
+                                .getType(0)
+                                .cast<fir::BoxProcType>()
+                                .getEleTy();
+        mlir::Value funcAddr = builder.create<fir::AbsentOp>(loc, funcTy);
+        mlir::Value charLen =
+            builder.createIntegerConstant(loc, builder.getCharacterLengthType(),
----------------
jeanPerier wrote:
> Can I ask you to move this code into a new FirOpBuilder helper `builder.genAbsentOp(loc, argTy)` that would detect that the argTy is a tuple via `fir::isCharacterProcedureTuple(argTy)`, and use `fir::factory::createCharacterProcedureTuple` instead of `createBoxProcCharTuple` to create the tuple?
> 
> That way, it can be reused in other places that also suffer from this bug, like the equivalent HLFIR code pieces here: 
> - https://github.com/llvm/llvm-project/blob/91cbc3f2d83dfcf064238e807b47c58279509ff7/flang/lib/Lower/ConvertCall.cpp#L1047
> - https://github.com/llvm/llvm-project/blob/91cbc3f2d83dfcf064238e807b47c58279509ff7/flang/lib/Lower/ConvertCall.cpp#L764
Right, I'll make this change and fix these HLFIR code pieces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144743