[Chapel Merge] Ensure that secondary/tertiary methods are defined

Notifications Commits
1

Branch: refs/heads/main
Revision: c20ecba
Author: bradcray
Link: Unavailable
Log Message:

Merge pull request #20262 from bradcray/error-for-methods-on-non-types

Ensure that secondary/tertiary methods are defined on types, not values

[reviewed by @vasslitvinov — thanks!]

In issue #20259, user @ghbrown pointed out that Chapel currently allows tertiary
methods to be defined on values rather than types (which has the same effect as
defining the method on the type itself, but isn't something I think we ever intended
to support). This PR closes that loop by tightening up the resolution of the method
receiver to ensure that it's a type and to complain otherwise.

Though I think this is better than what was on main, it had a few aspects of it
that were a little disconcerting and worthy of additional looks / future work:

  • First, it seems that we call getValType() on arbitrary formal type expressions without
    checking to see whether the thing we're calling it on is a type or not. This makes me
    wonder where else in the compiler value expressions can be used where a type is
    expected without warning, and whether we have (or should have) a method that says
    "convert this expression which I think is a type into its corresponding value type and
    complain at me if it's not actually a type" (where getValType() currently also seems to
    support "get the value type of this arbitrary expression", which makes sense in that
    sort of context).

  • Next, it seems as though probably this check could/should be applied to all formal
    expressions rather than just the _this formal as I'm doing here. However, when I
    tried this, it complained about true not being a type in:

      private proc externFunc(param s: string, type T, param explicit=true) param {

    which suggests to me that we may just abuse the typeExpr field of formals at some
    point in the compiler by changing explicit = true into explicit: true = true (rather
    than (explicit: true.type = true)?

In the discussion on this PR, Vass provides some more context for the status quo
and options for improving it, though I'm not pursuing those, and would suggest
we just build dyno to be better in this regard rather than trying to fix the legacy
logic.

Resolves #20259.

Modified Files:
A test/types/records/bugs/methodOnValue.chpl

A test/types/records/bugs/methodOnValue.good
M compiler/AST/FnSymbol.cpp

Compare: https://github.com/chapel-lang/chapel/compare/86cdfc22e94d...c20ecba57c5a