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

[Jean Perier via Phabricator via flang-commits]

jeanPerier added a comment.

Thanks a lot for the fix!



================
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(),
----------------
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


================
Comment at: flang/lib/Lower/ConvertExpr.cpp:2481
+            builder.createIntegerConstant(loc, builder.getCharacterLengthType(),
+                                          fir::CharacterType::singleton());
+        caller.placeInput(
----------------
I would use fir.undefined or zero rather than 1 for the length of an absent character procedure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144743