[flang-commits] [PATCH] D123927: [flang] Do not ICE on recursive function definition in function result

[Daniil Dudkin via Phabricator via flang-commits]

unterumarmung added inline comments.


================
Comment at: flang/include/flang/Semantics/symbol.h:698
+  inline const DeclTypeSpec *GetTypeImpl(VisitedSymbols &visited) const;
+  inline int RankImpl(VisitedSymbols &visited) const {
+    if (visited.contains(this)) {
----------------
klausler wrote:
> Your changes make Symbol::Rank() and Symbol::GetType() more expensive to call and liable to allocate/deallocate memory for all programs.  Would it be sufficient to just use an integer argument to limit the call depth?  That would avoid a crash in erroneous tests without affecting compilation for correct programs.
That's a great idea! But, to be honest, I'm having a hard time coming up with an initial value for such argument. Is there a limit for number of nested calls of these functions for correct programs? Or, maybe, we should use some //big enough// value that //probably// is not encountered in correct programs, yet lets us avoid the stack overflow?


================
Comment at: flang/lib/Semantics/check-declarations.cpp:1789
     if (!details) {
+      if (generic.test(Symbol::Flag::Function)) {
+        Characterize(generic);
----------------
klausler wrote:
> When `generic` is not `GenericDetails`, what is it?
Well, the `generic` was written not by me. If I understand correctly, `generic` is a `Symbol` that meant to have the `GenericDetails`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123927