[flang-commits] [PATCH] D123947: [flang] Fix ICE for passing a label for non alternate return arguments

[Kiran Chandramohan via Phabricator via flang-commits]

kiranchandramohan added inline comments.


================
Comment at: flang/lib/Semantics/check-call.cpp:678-684
+            if (arg.isAlternateReturn()) {
+              auto restorer{messages.SetLocation(
+                  arg.sourceLocation().value_or(messages.at()))};
+              messages.Say("Label '%d' can't be associated with %s"_err_en_US,
+                  arg.GetLabel(), dummyName);
+              return;
+            }
----------------
unterumarmung wrote:
> kiranchandramohan wrote:
> > Isn't this error applicable to DummyProcedure too?
> I thought about it too! But there's already an error for this code:
> ```fortran
> module m
>     contains
>       subroutine s3(v)
>         integer, intent(in) :: v
>       end subroutine s3
>       subroutine s(x)
>         procedure(s3) :: x
>       end
>       subroutine s2
>         call s(*42)
>         42 stop
>       end
>   end module m
> ```
> ```
> error: Semantic errors in a.f90
> ./a.f90:10:9: error: Assumed-type argument may not be forwarded as procedure dummy argument 'x='
>           call s(*42)
>           ^^^^^^^^^^^
> ```
> Yet, the error message is not totally correct. Should I add the check to DummyProcedure also?
Yes, i think it is useful here as well.


================
Comment at: flang/lib/Semantics/check-call.cpp:683
+                  arg.GetLabel(), dummyName);
+              return;
+            }
----------------
kiranchandramohan wrote:
> Nit: Sometimes we prefer not to use early returns. Please wait for another opinion.
Can you remove the early return by using a nested if?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123947