[flang-commits] [PATCH] D84290: [Flang] Fix for the scenario when type guard has intrinsic type specification and Selector is NOT unlimited Polymorphic.

[Pete Steinfeld via Phabricator via flang-commits]

PeteSteinfeld added a comment.

Thanks for working on this!



================
Comment at: flang/lib/Semantics/check-select-type.cpp:84
+                    !selectorType_.IsUnlimitedPolymorphic()) { // C1162
+                  context_.Say(stmt.source,
+                      "If selector is not Unlimited Polymorphic, "
----------------
inderjeet-hcl wrote:
> sameeranjoshi wrote:
> > I tried to minimize the use of passing `stmt` as a parameter by using `parser::FindSourceLocation(typeSpec)` it was strange that I couldn't see the source location information printed.
> > 
> > Does `parser::FindSourceLocation` fail in some scenarios @PeteSteinfeld ?
> > If that's the case using `stmt` is completely fine here.
> Kindly refer following case when line number information is not output in error message. 
> In case of intrinsic types 'integer','character' etc. parser::FindSourceLocation(typespec) is returning empty:
> 
> [root at localhost Github_compiler]# f18 selecttype.f90 
> error: The type specification statement must have LEN type parameter as assumed
> f18: semantic errors in selecttype.f90
> [root at localhost Github_compiler]# cat -n selecttype.f90 
>      1	class(*),allocatable :: cptr
>      2	
>      3	select type(cptr)
>      4	 !ERROR: The type specification statement must have LEN type parameter as assumed
>      5	 type is(character) !<-- assumed length-type
>      6	end select
>      7	end
> 
It looks like you could simplify this by just passing the `TypeGuardStmt`.  You can use it to find the `TypeGuardStmt::Guard` and the source location of the `TypeGuardStmt`.


================
Comment at: flang/lib/Semantics/check-select-type.cpp:85
+                  context_.Say(stmt.source,
+                      "If selector is not Unlimited Polymorphic, "
+                      "intrinsic type specification must not be specified "
----------------
inderjeet-hcl wrote:
> sameeranjoshi wrote:
> > Why are `u` and `p` capitalized?
> > Read more on error message to user guideline:
> > https://github.com/llvm/llvm-project/blob/master/flang/documentation/C%2B%2Bstyle.md#error-messages
> Thanks for sharing document reference. I will update it as follows:
>                   context_.Say(stmt.source,
>                       "If selector is not unlimited polymorphic, "
>                       "intrinsic type specification must not be specified "
>                       "in type guard statement"_err_en_US);
> 
Here's what I would prefer for the message:
```
context_.Say(stmt.source,
    "If the selector is not unlimited polymorphic, "
    "an intrinsic type specification must not be specified "
    "in the type guard statement"_err_en_US);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84290