Consider this test case:
-------8<-------
{$mode ObjFPC}

// EXPECTED: 'Error: Identifier not found "result"'
// ACTUAL BUG #1: gets compiled
operator - (const L, R: Char) returned: Char;
begin
	result := 'Z'
end;
// EXPECTED: gets compiled
// ACTUAL BUG #2: 'Error: Duplicate identifier "result"'
operator / (const L, R: Char) result: Char;
begin
	result := 'Z'
end;
begin
end.
-------8<-------
The patch http://hg.blaise.ru/public/fpc/rev/ce20f68924a0 (attached) alters insert_funcret_local:
1) Extracts duplicate code into alias_funcret;
2) Avoids the second call to the pd.resultname getter;
3) Avoids copying strings pd.resultname^/pd.procsym.name to the local buffer hs;
4) Fixes the first bug: the alias Result should not be implicitly declared when another identifier is declared as the return value;
5) Fixes the second bug: even with {$modeswitch result+}, it should be possible to explicitly name a return value Result.
NB: The patch preserves the current suspicious behaviour of not slapping vo_is_result onto explicit names. More on that later.

If needed, I can file a bug report to document the user-visible change in behaviour.

Am 15.12.2020 um 20:00 schrieb

No, those two are in fact correct this way.
The modeswitch Result enables the use of Result as an alias for *any* routine that returns a result no matter if it's an ordinary function or an operator and for those no matter if an explicit result name is set or not.
Additionally the following will fail as well (this is allowed in mode Delphi however):

=== code begin ===

{$mode objfpc}
function Result: LongInt;
begin
end;
begin
end.

=== code end ===

Patch rejected as is due to the above point. If you want to address 1) to 3) you can do so however.

vo_is_result is only set on the Result alias by design (search for vo_is_result and you'll see a few locations where vo_is_result is checked while vo_is_funcret is not).

Also please use { ... } for comments, not // ...

Regards,
Sven

Sven Barth wrote

No, those two are in fact correct this way.

Is that the /team's/ consensus?

The modeswitch Result enables the use of Result as an alias for *any* routine …

Incorrect. The identifier Result does not alias the routine, it aliases the routine's return value.

… that returns a result no matter if an explicit result name is set or not.

I maintain that such approach is an orthogonality violation. In light that FPC provides a syntax for explicitly naming the return value, the consistent and conceptually simplest way to treat Result as the implicit/default name for that value, not as an alias to a name that cannot even be specified in most cases (non-operators).

Were the presented cases examined when the current behaviour was implemented? And, if so, what were the points defending it? Does anyone actually think that there is even one good theoretical or practical reason to reject the following? -------8<-------
{$modeswitch result+}

// ACTUAL: 'Error: Duplicate identifier "result"'
operator / (const L, R: Char) result: Char;
begin
	result := 'Z'
end;
-------8<-------
The compiler basically says: "You cannot name the return value Result because /I/ am naming the return value Result!". Quite ridiculous and not in the spirit of Pascal.

The following behaviour also fits my narrative rather nicely: -------8<-------
{$modeswitch result-}

// ACTUAL: 'Fatal: Syntax error, "identifier" expected but ":" found'
operator * (const L, R: Char): Char;
begin
	exit('Z')
end;
-------8<-------
Here, rightfully, the compiler does not threat these features as separate/orthogonal: "You opted out of the default name for the return value, do provide an explicit name."

Additionally the following will fail as well (this is allowed in mode Delphi however): function Result: LongInt;

That is not quite relevant for this discussion. A function and its return value are different entities.

To be correct: Result is not the name of the result value, it is an alias.

The following shows this:

Function MyResult : String;
begin
   Result:='';
   MyResult:='';
end;

You can still use the function name for the result, so "Result" is in fact an alias for the function name, which is the actual name for the result value.

But I do agree with your point that it should be allowed to explicitly name Result as Result for operators, even when the Result identifier is allowed.

It really makes no sense to me to forbid this.

Michael.

Blaise--- via fpc-devel schrieb am Mi.,
16. Dez. 2020, 10:14:

For non operators the routine's name *is* the name of the return variable.

For non operators the name of the real result value *is* the name of the routine and Result is the alias. Operators don't have names (in the classical sense, internally they have one of course), thus for the modes without Result modeswitch a way had to be introduced to specify the name of the variable containing the result.

I doubt that was examined back then, but right now the behavior is consistent and I see no need to change it.

Yes, it is relevant for this discussion, because it's the same piece of code in the compiler that handles this.

Regards,
Sven

On Wed, 16 Dec 2020 10:24:39 +0100 (CET)
Michael Van Canneyt via fpc-devel

That is not entirely correct.

Function MyResult : String;
begin
   Result:=Result; // the current result
   MyResult:=MyResult; // recursive call
end;

Mattias

Michael Van Canneyt wrote

To be correct: Result is not the name of the result value, it is an alias.

I did not dispute that. The important point here is: "an alias" to /what/?

You can still use the function name for the result, so "Result" is in fact an alias for the function name

Result is not "an alias for the function name" because you cannot use Result to refer to the function:
-------8<-------
function Foo: Integer;
var X: function : Integer;
begin
	Result(); // ERROR
	X := Result; // ERROR
end;
-------8<-------

the function name, which is the actual name for the result value.

I can get behind this notion; however, within the function, that name is overloaded. Thus, we should not say, like Sven did, that Result aliases the function (name); we should say, like I did, that it aliases the return value (name).

But, conceptually, I would rather look at the function name resolving to the return value as a backward-compatible alias, and say that Result is the true (default) name for the return value. Such notion fits modern usage.

I do agree with your point that it should be allowed to explicitly name Result as Result for operators, even when the Result identifier is allowed.

The score thus far: 1:1.

So, let's say that:

The function result can be assigned using 2 identifiers:
* The Result identifier.
* The name of the function.

> I do agree with your point that it should be allowed to explicitly name Result as Result for operators, even when the Result identifier is allowed. The score thus far: 1:1.

Well, it's not exactly a contest.

We can of course consider changing the current behaviour in the future, as far as I know there is no urgency in this ATM. There are other, more urgent tasks...

Michael.

It is a _local_ alias for the name of the _currently_executed_ function which is valid just within the scope of that function. When invoking the function recursively, you refer to _another_ instance of that function and thus the alias is not valid at that point in time, because it becomes an alias again only when the recurrently called function starts.

Thus it's equivalent to the following example (it compiles due to {$MODE TP}; in other modes the local can be replaced with another name with the same effects):

{$MODE TP}
  function Foo: Integer;
  var
   X: Integer absolute Foo;
  begin
   X := 123;
  end;
begin
  WriteLn (Foo);
end.

Modern or not - use of the function name for assignment of the return value is part of Pascal definition, isn't it? What's the point of changing the "true name"?

Tomas

Michael Van Canneyt via fpc-devel schrieb
am Mi., 16. Dez. 2020, 12:47:

We can of course consider changing the current behaviour in the future, as far as I know there is no urgency in this ATM. There are other, more urgent tasks...

I agree. Please concentrate on anonymous functions and don't change things that don't need to be changed *now*.

Regards,
Sven