[PATCH] D88313: [flang] Failed call to CHECK() for call to ASSOCIATED(NULL())

[Peter Klausler via Phabricator via llvm-commits]

klausler added inline comments.


================
Comment at: flang/include/flang/Evaluate/characteristics.h:233
   static std::optional<DummyArgument> FromActual(
-      std::string &&, const Expr<SomeType> &, FoldingContext &);
+      std::string &&, const Expr<SomeType> &, FoldingContext &, bool);
   bool IsOptional() const;
----------------
Magic flag arguments should be avoided.  If you want special-case handling of some input in some situation, consider adding a wrapper function with the magic that defaults to the original function otherwise.  But maybe the new special behavior doesn't have to be special -- what would break if you just changed the treatment of `NULL` for all cases?


================
Comment at: flang/include/flang/Evaluate/type.h:204
+    TypelessKind = -1, // BOZ actual argument to intrinsic function or pointer
+                       // argument to ASSOCATED
     ClassKind = -2, // CLASS(T) or CLASS(*)
----------------
`ASSOCIATED`


================
Comment at: flang/lib/Evaluate/characteristics.cpp:361
+                : std::optional<DummyArgument>{};
+            //            return std::optional<DummyArgument>{};
+          },
----------------
Obsolete comment?


================
Comment at: flang/lib/Evaluate/intrinsics.cpp:87
     addressable, // for PRESENT(), &c.; anything (incl. procedure) but BOZ
+    pointerType, // for ASSOCIATED
 )
----------------
If it's only meant to be used for `NULL()`, please use a more specific name.


================
Comment at: flang/lib/Evaluate/intrinsics.cpp:286
+        {{"pointer", AnyPointer, Rank::known},
             {"target", Addressable, Rank::known, Optionality::optional}},
         DefaultLogical, Rank::elemental, IntrinsicClass::inquiryFunction},
----------------
Can the optional `TARGET=` argument be `NULL(MOLD=)` or even just `NULL()`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88313