Finally some movement is happening on implicit function specialization and I'm almost finished now except some questions about precedence have been raised again. Initially I thought we decided on non-generic functions taking precedence in the case of *any* name collisions (the original thread https://lists.freepascal.org/pipermail/fpc-pascal/2018-December/055225.html) but Sven is saying now that this isn't the case (see the remarks on the bug report https://bugs.freepascal.org/view.php?id=35261). I'm asking this here not to go over Svens head but in hopes to get some answers quicker (it can take us weeks sometimes to round trip even simple questions).

Currently what I implemented is that in the case below non-generic Test() will take precedence even though Test<T> could be specialized and indeed even comes after. My questions:

1) What is required for Delphi compatibility? I never used Delphi and I thought we decided this initially for Delphi compatibility. Of course we can make a Delphi mode only option if we need to.

2) Svens final remarks on the bug tracker are "Right now your code will pick the existing String overload even if specializing the generic might be the better choice. If the user really wants the String one (or if the specializing the generic does not work) the user can still force the String overload by casting the parameter to a String.". I'm confused about this because Test(String) and Test<String> are both identical and thus I don't see what is the "better choice".

Personally I feel like we should fallback to the non-generic function as a way to resolve ambiguity but I can also see why Test<T> should take precedence simply because it comes after Test().

================

{$mode objfpc}{$H+}
{$modeswitch IMPLICITFUNCTIONSPECIALIZATION}
function Test(const aStr: String): LongInt;
begin
  Result := 1;
end;
generic function Test<T>(aT: T): LongInt;
begin
  Result := 2;
end;
begin
  Writeln(Test('Hello World'));
end.

================

And just to be clear even if the name is the same but parameters do not match the generic function will be selected. For example below Test<T> will be specialized because Test() doesn't have a compatible signature.

================

function Test(const aStr: String): LongInt;
begin
  Result := 1;
end;
generic function Test<T>(aT: T; aInt: Integer): LongInt;
begin
  Result := 2;
end;
begin
  Writeln(Test('Hello World', 1));    // prints "2"
end.

================

Regards,
Ryan Joseph

In the example you posted below, I agree with you, but that is not what I said. Look at my example again:

=== code begin ===

program timplicitspez;
{$mode objfpc}{$H+}
{$modeswitch IMPLICITFUNCTIONSPECIALIZATION}
function Test(const aStr: String): LongInt;
begin
   Result := 1;
end;
generic function Test<T>(aT: T): LongInt;
begin
   Result := 2;
end;
operator := (aArg: LongInt): String;
begin
   { potentially expensive conversion }
   Result := '';
end;
begin
   Writeln(Test('Hello World'));
   Writeln(Test(42));
end.

=== code end ===

The important part here is the operator overload. If the generic function would not exist then the compiler would simply call the operator overload to convert the 42 to a String and call the function with the string overload. While in this example the conversion is essential a no-op in reality it might be more complex and expensive thus it might be better to use the implicit specialization of the 42 as is (in this case it would be a LongInt I think). Right now there is no possibility to enforce the use of the implicit specialization.

In this specific case the two functions also are *not* ambigous, because for the non-generic Test the parameter requires an implicit conversion, but the implicit specialization does not. For example if there would be a "Test(aArg: LongInt)" instead of the generic the compiler would pick that instead of the string one. So if you move the check for generic vs. non-generic to the end of is_better_candidate all the other rules to determine this will take precedence.

And to pick the non-generic one can do this in the above example: Test(String(42)), because here the type your implicit specialization code will receive will be String already and thus the generic vs. non-generic check will catch it and prefer the non-generic one.

Also Delphi agrees with me:

=== code begin ===

program timplspez;
{$APPTYPE CONSOLE}
uses
   SysUtils;
type
   TFoo = record
     f: LongInt;
     class operator Implicit(aArg: LongInt): TFoo;
   end;
   TTest = record
     class function Test(aArg: TFoo): LongInt; overload; static;
     class function Test<T>(aArg: T): LongInt; overload; static;
   end;
class operator TFoo.Implicit(aArg: LongInt): TFoo;
begin
   Result.f := aArg;
end;
class function TTest.Test(aArg: TFoo): LongInt;
begin
   Result := 1;
end;
class function TTest.Test<T>(aArg: T): LongInt;
begin
   Result := 2;
end;
var
   f: TFoo;
begin
   f := 21;
   Writeln(TTest.Test(f));
   Writeln(TTest.Test(42));
   Writeln(TTest.Test(TFoo(42)));
   Readln;
end.

=== code end ===

=== output begin ===

1
2
1

=== output end ===

This should answer both your points, cause they're related.

Regards,
Sven

On Apr 6, 2021, at 12:57 PM, Sven Barth wrote: In this specific case the two functions also are *not* ambigous, because for the non-generic Test the parameter requires an implicit conversion, but the implicit specialization does not. For example if there would be a "Test(aArg: LongInt)" instead of the generic the compiler would pick that instead of the string one. So if you move the check for generic vs. non-generic to the end of is_better_candidate all the other rules to determine this will take precedence.

So the root of the problem is that we have no way to choose the generic function via explicit casting? That makes sense and presumably I can use the final result of is_better_candidate to determine this? In your example:

Writeln(Test('Hello World'));			// is_better_candidate res = -1
Writeln(Test(42));					// is_better_candidate res = 1
Writeln(Test(String(42)));			// is_better_candidate res = -1

I'm struggling to see how the operator influenced the result. Res is -1 in both cases so how do we know which Test we want to call?

Regards,
Ryan Joseph

On Apr 6, 2021, at 12:57 PM, Sven Barth wrote: In the example you posted below, I agree with you, but that is not what I said. Look at my example again:

Also could you please verify that $H+ isn't causing problems? The string literal 'Hello World' is a short string but "String" is an AnsiString so that may influence the results you're expecting. However even without the operator I don't know how to delineate between the explicit cast vs the non-explicit cast.

	Writeln(Test('Hello World'));
	Writeln(Test(String('Hello World')));

These both appear to be equal as far as is_better_candidate is concerned.

Regards,
Ryan Joseph

You simply need to put your check for generic vs. not-generic after the check for ordinal_distance inside is_better_candidate instead of at the start of the function.

The compiler is free to use implicit operator overloads when determining the overload to pick.

When the compiler sees the first call to Test it will have the non-generic Test(String) in its candidate list as well as Test<String>(String) due to the implicit specialization. Here it will use the non-generic one.

In the second case the compiler will have the non-generic Test(String) due to the implicit operator as well as Test<LongInt>(LongInt) due to the implicit specialization. Here it will pick the generic one, because a call without a type conversion is considered better.

In the third case the will will contain Test(String) as well as Test<String>(String) due to the parameter being explicitely casted to String with the compiler again picking the non-generic one.

Without the explicit cast by the user the compiler would have picked the Test<LongInt>, but using a cast to the type of non-generic function one is able to enforce the non-generic function (for example to avoid an error during specialization if the Test function would not be able to handle the LongInt type).

Regards,
Sven

First of *you* need to make sure that the type you pick for the generic parameter for a string constant is the correct one matching the $H+ directive as well as the UnicodeString modeswitch (and if the string constant contains WideChar elements it needs to be a UnicodeString anyway). That's where you currently search for the SHORTSTRING type.

Then you only need to make sure that the check for generic vs. non-generic is in place as I said in the other mail and the compiler will do the rest for you. You don't need to do *anything*. Your task is merely to provide the compiler with the suitable overload candidates and a way to check their order and it will already do all the rest.

Regards,
Sven

On Apr 6, 2021, at 11:34 PM, Sven Barth wrote: In the second case the compiler will have the non-generic Test(String) due to the implicit operator as well as Test<LongInt>(LongInt) due to the implicit specialization. Here it will pick the generic one, because a call without a type conversion is considered better.

I finally get what you're saying and get the correct results now after moving my check inside the if/then block. I didn't realize that the operator overload was causing a type conversion that the overloading system knew about. So the non-generic still wins unless the type conversion happened due to the operator overload and then the generic wins.

	Writeln(Test('Hello World'));			// Test(String)
	Writeln(Test(42));					// Test<String>
	Writeln(Test(String(42)));			// Test(String)

As for $H+ do we really want string literals likes 'ABC' to change? I wouldn't think so. Here's my string literal type symbol conversion in create_unamed_typesym I'm using now:

              case tstringdef(def).stringtype of
                st_shortstring:
                  newtype:=search_system_type('SHORTSTRING');
                st_longstring,
                st_ansistring:
                  newtype:=search_system_type('ANSISTRING');
                st_widestring:
                  newtype:=search_system_type('WIDESTRING');
                st_unicodestring:
                  newtype:=search_system_type('UNICODESTRING');
              end;

'Hello World' is parsed as st_shortstring so we use System.ShortString for specialization. Given that I don't think I need to do anything with $H+.

Regards,
Ryan Joseph

Good. :)

Yes, we want to change that for two reasons:
- the constant string might be larger than 255 characters
- ShortString is worse for passing as a by-value parameter (which will be the default after all) than AnsiString or UnicodeString as ShortString is completely copied while Ansi-/UnicodeString are only references with reference count adjustments

Also your check is wrong as the def that returns True for is_stringlike might not be a stringdef. In the case of a constant string it's an arraydef with ado_ConstantString in arrayoptions (and that's how you can detect constant strings which *need* this treatment).

Regards,
Sven

Ok, so with $H+ constant strings will be specialized as AnsiStrings. And there is another unicode string mode I should do a similar thing with? Also if you happen to know where I can get the state of $H+ that would be helpful otherwise I need to track it down in the debugger. :)

Regards,
Ryan Joseph

With the requested changes I believe some precedence rules have changed. These both should be "Can't determine which overloaded function to call" errors or the non-generic should take precedence because the functions are ambiguous (by appearance at least). Currently the compiler thinks DoThis<T> is better than the non-generic and this may be because it was specialized as DoThis<ShortInt> because the parameter of "1" is a ShortInt.

What should the rule be here?

=====================

procedure DoThis(a: word; b: word);
begin
end;
generic procedure DoThis<T>(a:T; b: word);
begin
end;
begin
  DoThis(1,1); // DoThis<T>
end.

=====================

generic procedure DoThis<T>(a:T; b: word);
begin
end;
generic procedure DoThis<T>(a: word; b: T);
begin
end;
begin
  DoThis(1,1); // Can't determine which overloaded function to call
end.

=====================

Regards,
Ryan Joseph

On Apr 7, 2021, at 1:56 PM, Ryan Joseph wrote: Ok, so with $H+ constant strings will be specialized as AnsiStrings. And there is another unicode string mode I should do a similar thing with? Also if you happen to know where I can get the state of $H+ that would be helpful otherwise I need to track it down in the debugger. :)

I think I got this part figured out (see function below). I'm going to upload another patch and a bunch of unit tests on the bug tracker but I'm leaving my latest ambiguous function call as-is until further notice. it's sneaky like it is but it follows rules which you can manipulate using casting.

==============================

function create_unamed_typesym(def:tdef): tsym;
        var
          newtype: tsym;
        begin
          newtype:=nil;
          if is_stringlike(def) then
            begin
              if (def.typ=arraydef) and (ado_isconststring in tarraydef(def).arrayoptions) then
                newtype:=nil
              else
                case tstringdef(def).stringtype of
                  st_shortstring:
                    newtype:=nil;
                  st_longstring,
                  st_ansistring:
                    newtype:=search_system_type('ANSISTRING');
                  st_widestring:
                    newtype:=search_system_type('WIDESTRING');
                  st_unicodestring:
                    newtype:=search_system_type('UNICODESTRING');
                end;
              { not better string type was found so chose the default string type }
              if newtype=nil then
                begin
                  if (cs_refcountedstrings in current_settings.localswitches) then
                    begin
                      if m_default_unicodestring in current_settings.modeswitches then
                        newtype:=search_system_type('UNICODESTRING')
                      else
                        newtype:=search_system_type('ANSISTRING');
                    end
                  else
                    newtype:=search_system_type('SHORTSTRING');
                end;
            end
          else
            begin
              newtype:=ctypesym.create(def.typename,def);
              newtype.owner:=def.owner;
            end;
          if newtype=nil then
            internalerror(2021020904);
          result:=newtype;
        end;

Regards,
Ryan Joseph

In Delphi this takes the non-generic. But I think this comes into a region where things are not clearly documented, thus it's hard to get things "right". In theory one could move the generic check in front of the ordinal check and it should work, but it could get wonky again if you pass e.g. a LongInt into this (Delphi keeps insisting on using the non-generic with a LongInt constant parameter even though if you have two non-generic overloads it will complain that it violates the range of Word and ShortInt... go figure... - with a LongInt variable it will use the generic)

Let it be as it is now... this will only result in headaches either way.

This is correct. Delphi behaves the same here (and it would be the same in the non-generic case with T being replaced by ShortInt).

Regards,
Sven

1. you should not blindly assume that the def is a stringdef if it's not an arraydef; at least use an internalerror to protect against problems here
2. if it's really a stringdef and the return type is st_shortstring you should indeed use SHORTSTRING (it's only constant strings which are a bit more, let's say "dynamic")
3. do an internalerror for st_longstring as those are currently not implemented
4. due to 2. you can move the case of newtype=nil into the if-clause with the arraydef

Otherwise, yes, the check for the string type is correct.

Regards,
Sven

I didn't know how constant strings we identified until just now so I can correct that. Can we make "(def.typ=arraydef) and (ado_isconststring in tarraydef(def).arrayoptions)" into a function in defutils.pas and call it is_constant_string? That would have been easily visible to me and lead me in the right direction from the start. I also had problems with array literals like [1,2,3] which there is no clear utility function for and prompted me to make is_array_literal() which is private right now (and probably not very correct in design). Moving that to a public space would be sensible also I think.

Regards,
Ryan Joseph

That is because before the introduction of type helpers such functions weren't really needed. Other mechanisms caught such constants, but for both type helpers and these implicit specializations it's hard to do it another way.

Regards,
Sven

On Apr 8, 2021, at 11:37 PM, Sven Barth via fpc-devel wrote: That is because before the introduction of type helpers such functions weren't really needed. Other mechanisms caught such constants, but for both type helpers and these implicit specializations it's hard to do it another way.

Ok then I'll add a is_constant_string function for later use.

What about this function I'm using, should it be in defutils.pas also? I need this so I can distinguish "unnamed array literals" like ['a','b','c'] from short strings like 'abc'. They may be internally identical to arrays but conceptionally they are different. Not sure about the naming so I chose "array literal" but "anonymous array" would make sense also.

      function is_array_literal(def:tdef): boolean;
        begin
          result := (def.typ=arraydef) and not is_constant_string(def);
        end;

Btw, this block (from create_unamed_typesym) could be a useful helper function in tstringdef, such as "get_default_system_type". I needed something similar to get the char type for a string def (and added that method already) so this is another logical extension to that.

case tstringdef(def).stringtype of
              st_shortstring:
                newtype:=search_system_type('SHORTSTRING');
              { st_longstring is currently not supported but
                when it is this case will need to be supplied }
              st_longstring:
                internalerror(2021040801);
              st_ansistring:
                newtype:=search_system_type('ANSISTRING');
              st_widestring:
                newtype:=search_system_type('WIDESTRING');
              st_unicodestring:
                newtype:=search_system_type('UNICODESTRING');
            end

Regards,
Ryan Joseph

I just realized one more type introspection related issue. This currently will specialize as DoThis<ShortInt> because the array constructor element type is "shortint" as is derived from the first element 1. This of course is not correct so I'd like to reject "array of const constructors" but I don't see how to know what when I get this type.

I thought the tarraydef would have ado_IsArrayOfConst set but it does not, so how do I know the array constructor has multiple element types?

generic procedure DoThis<T>(a: array of T);
begin
end;

begin
DoThis([
1,
'string',
'c',
TObject.Create
])
end.

Regards,
Ryan Joseph

This will lead to false assumptions, cause it will return True for *any* arraydef that's not a constant string.

Possibly, yes...

You could provide the various utility functions in a separate patch.

Regards,
Sven

Well I'm going to use them for this patch so they would all be batched together.

Any idea about the "array of const" issue I raised in the other email? Once that's done I'll submit another very patch. Getting very close now unless I missed some test cases...

Regards,
Ryan Joseph

You mean what you did for is_array_literal? A pure array constructor can be found with is_array_constructor, though it might be better to use is_open_array, cause someone might pass an open array parameter to a generic function (those will be unnamed as well).

Regards,
Sven

On Apr 9, 2021, at 4:31 PM, Sven Barth via fpc-devel wrote: You mean what you did for is_array_literal? A pure array constructor can be found with is_array_constructor, though it might be better to use is_open_array, cause someone might pass an open array parameter to a generic function (those will be unnamed as well).

Maybe the email didn't go through? How to identify this "array of const constructor" type. I want to reject these for implicit specialization but I can't identity them.

DoThis([
1,
'string',
'c',
TObject.Create
])
Regards,
Ryan Joseph

You can't really differentiate these from array constructors with just one type, because the type is only really determined once the overload is chosen as that kind of array could be an array of const, an array of Variant or maybe even an array of LongInt if there are suitable operator overloads in scope. So, yeah, it's kinda hard to decide what type to use...

Regards,
Sven

*Now* your other mail arrived. ;)

As an additional note: if you take a look at
tarrayconstructornode.pass_typecheck you can see that the array type always has the ado_IsConstructor set and if it contains of incompatible types the ado_IsVariant is set as well. So if ado_IsVariant is *not* set, then you can rely on the elementdef of the arraydef.

Regards,
Sven

On Apr 10, 2021, at 6:54 AM, Sven Barth wrote: As an additional note: if you take a you can see that the array type always has the ado_IsConstructor set and if it contains of incompatible types the ado_IsVariant is set as well. So if ado_IsVariant is *not* set, then you can rely on the elementdef of the arraydef.

I checked before and here's what I got. Maybe pass_typecheck hasn't been called yet? If not I'll have to reproduce that code and determine how it knows the elements are not uniform. Thanks.

elecount:4
typesym:{Array Of Const/Constant Open} Array of ShortInt ado_IsConvertedPointer: FALSE
ado_IsDynamicArray: FALSE
ado_IsVariant: FALSE
ado_IsConstructor: TRUE
ado_IsArrayOfConst: FALSE
ado_IsConstString: FALSE
ado_IsBitPacked: FALSE
ado_IsVector: FALSE
ado_IsGeneric: FALSE

                  writeln('elecount:',tarraydef(caller_def).elecount);
                  writeln('typesym:',tarraydef(caller_def).typesym.realname);
                  writeln('ado_IsConvertedPointer: ',ado_IsConvertedPointer in tarraydef(caller_def).arrayoptions);
                  writeln('ado_IsDynamicArray: ',ado_IsDynamicArray in tarraydef(caller_def).arrayoptions);
                  writeln('ado_IsVariant: ',ado_IsVariant in tarraydef(caller_def).arrayoptions);
                  writeln('ado_IsConstructor: ',ado_IsConstructor in tarraydef(caller_def).arrayoptions);
                  writeln('ado_IsArrayOfConst: ',ado_IsArrayOfConst in tarraydef(caller_def).arrayoptions);
                  writeln('ado_IsConstString: ',ado_IsConstString in tarraydef(caller_def).arrayoptions);
                  writeln('ado_IsBitPacked: ',ado_IsBitPacked in tarraydef(caller_def).arrayoptions);
                  writeln('ado_IsVector: ',ado_IsVector in tarraydef(caller_def).arrayoptions);
                  writeln('ado_IsGeneric: ',ado_IsGeneric in tarraydef(caller_def).arrayoptions);

Regards,
Ryan Joseph

On Apr 10, 2021, at 9:18 AM, Ryan Joseph wrote: I checked before and here's what I got. Maybe pass_typecheck hasn't been called yet? If not I'll have to reproduce that code and determine how it knows the elements are not uniform. Thanks.

Just checked and pass_typecheck is called before overloading but ado_IsVariant is simply never set for that array. In tarraydef.GetTypeName you can see that "array of const" is associated with many flags so maybe we need to make a new flag which means "non-uniform elements"? I could probably easily build that into pass_typecheck for array constructors.

...
          if (ado_isarrayofconst in arrayoptions) or
                 (ado_isConstructor in arrayoptions) then
           begin
             if (ado_isvariant in arrayoptions) or ((highrange=-1) and (lowrange=0)) then
               GetTypeName:='Array Of Const'
             else
               GetTypeName:='{Array Of Const/Constant Open} Array of '+elementdef.typename;
           end

Regards,
Ryan Joseph

Any word on what I should do about this? If those flags present are not sufficient I'll add another flag but if that's not acceptable I'll simply have to allow the user to specialize with these array types, even though they will fail later one when the function is selected during overloading. Once I know that I'll submit another patch including the other changes that were requested.

Regards,
Ryan Joseph

Looking at it, it could be that there is a bug in
tarrayconstructornode.pass_typecheck that hasn't really surfaced yet... I'll have to look at that first, but I don't know when I'll have the time for that.

Regards,
Sven

On Apr 11, 2021, at 3:33 PM, Sven Barth wrote: Looking at it, it could be that there is a bug in tarrayconstructornode.pass_typecheck that hasn't really surfaced yet... I'll have to look at that first, but I don't know when I'll have the time for that.

sure I'll just leave it as is for now then. By the time the overloading happens it must know the array constructor is array of const but it should ideally be known by the time tarrayconstructornode.pass_typecheck is executed.

Regards,
Ryan Joseph

Had a bit of time to can try the attached patch. You can then check for both ado_IsConstructor and ado_IsArrayOfConst to detect such a mixed array.

Regards,
Sven

On Apr 14, 2021, at 2:33 PM, Sven Barth wrote: Had a bit of time to can try the attached patch. You can then check for both ado_IsConstructor and ado_IsArrayOfConst to detect such a mixed array.

It works but it thinks this array is array of const also so it's too strict I believe.

['aaa', 'bbb'];

Regards,
Ryan Joseph

Well, then I'll have to improve the check. But for now you can continue, right?

Regards,
Sven

On Apr 14, 2021, at 11:39 PM, Sven Barth wrote: Well, then I'll have to improve the check. But for now you can continue, right?

I can continue but if I include the check some tests will fail. Currently I've only made some changes in create_unamed_typesym and now this check to reject array of const. Everything else is done AKAIK unless we discover more bugs in the tests (which I've uploaded on the bug tracker).

Regards,
Ryan Joseph

About this, shouldn't we just be doing this? Any array constructor that has elements which are which are incompatible is "array of const"? from tarrayconstructornode.pass_typecheck:

                 if eq=te_incompatible then
                   diff:=true;

Regards,
Ryan Joseph

Ryan Joseph via fpc-devel schrieb am Fr.,
16. Apr. 2021, 00:38:

Yes, do that for now.

Regards,
Sven

On Apr 16, 2021, at 2:44 AM, Sven Barth via fpc-devel wrote: Yes, do that for now.

Got this all integrated and put up the changes to https://bugs.freepascal.org/view.php?id=35261. Now I'm waiting for another final review. :)

Regards,
Ryan Joseph

On Apr 16, 2021, at 11:35 AM, Ryan Joseph wrote: Got this all integrated and put up the changes to https://bugs.freepascal.org/view.php?id=35261. Now I'm waiting for another final review. :)

The next thing to do now is to handle a memory leak. From the bug tracker:

You essentially need to make sure the symbols become part of the specialization that is picked in the end (cause you create the symbols in make_param_list only once, but you might use them for multiple generic overloads of which at most one will be picked). What you might be able to do is the following (have not looked at it in detail yet): - those symbols you directly add to genericparams you ensure that their owner is the specialization right away (after generate_specialization_phase1) - those symbols you add to callerparams you'll probably have to bubble up to htypechk and have that somehow add these syms to the final specialization if it is picked and free them otherwise

My response:

And with that I'm stuck. :) I don't know how the symbols in the compiler are memory managed but I don't see setting their owner is helping them get freed. I've tried setting their owner to the procdef.owner being specialized but that does't do anything either. I guess I don't understand what adding them to "the final specialization" means so please clarify.

My naive assumption would be to add them to a list in tspecializationcontext and free them when the content is freed because I don't think the symbols are used after the implicit specialization is finished and a proc def is produced, but I may be wrong about that.

Regards,
Ryan Joseph

I just noticed a week has passed on this. Sven, do you have a reply to my previous message to clarify a little? I could make a guess at what you want but I'll probably just waste more time doing the wrong thing. :P

Regards,
Ryan Joseph

I found something sneaky I'd like to confirm before I decide what to do about it.

1) "T" in TAnyClass<T> is specialized as Integer from the first parameter with TSomeClass (which is TAnyClass<Integer>).
2) "U" is getting specialized as String by looking at the parameters in Compare() in which "U"(the second generic parameter) is String.

This specializes the procedure correctly but it uses a very sneaky method which is very hard to discern. I feel like that if a generic parameter is already used (like T in specialize TCallback<T, U>) then no further attempt should be made to look at the parameters and in the example below "U" would not be found and the function would fail to implicitly specialize.

==============================

type
  generic TAnyClass<U> = class
    type TElem = U;
  end;
type
  TSomeClass = specialize TAnyClass<Integer>;
type
  generic TCallback<T, U> = function(a: T; b: U): integer;
function Compare(a: TSomeClass.TElem; b: string): integer;
begin
  result := 1;
end;
generic procedure DoThis<T, U>(aClass: specialize TAnyClass<T>; callback: specialize TCallback<T, U>);
begin
  callback(1, 'string');
end;
begin
  DoThis(TSomeClass.Create, @Compare);
end.

Regards,
Ryan Joseph

There is nothing sneaky if one defines that the parameters are evaluated left to right and those that are already found and used later on (like T) are simply fixed then thus in the example it uses the already found type. But if the picked type wouldn't mach, for example if TCallback<,> would be declared as TCallback<T: TObject; U> instead then trying to specialize the function would simply fail.

Regards,
Sven

You need to use ChangeOwner as well, but as I wrote you need to pay attention for which created symbol you do it at what time.

The ones you create in is_possible_specialization you need to add to the specialization you do in the following generate_implicit_specialization as those are only created for that specific specialization (though you also need to free it if you leave is_possible_specialization with False which you currently don't do).

The ones you create in make_param_list you need to pass back to htypechk so that they can either be added to the final specialization (if selected out of multiple ones) or to be freed if none of the ones created by that invocation to try_implicit_specialization is picked. That is because they would be shared by all the specializations.

Regards,
Sven

On May 8, 2021, at 12:04 PM, Sven Barth wrote: You need to use ChangeOwner as well, but as I wrote you need to pay attention for which created symbol you do it at what time.

Ok, maybe this is what I got wrong didn't use ChangeOwner. When you say "add to" what exactly do you mean? Please post a single line code snippet even. Thanks.

Regards,
Ryan Joseph

Ryan Joseph via fpc-devel schrieb am Sa.,
8. Mai 2021, 22:33:

Essentially it will boil down to sym.ChangeOwner(pd.parast)

However you need to keep the Owner (which is different from what you change with ChangeOwner) different as otherwise is_specialization of the procdef will not work correctly.

Regards,
Sven

I'm saving the storing symbols in tcallcandiates so they can be available when maybe_add_pending_specialization is called once the call node chooses the specialization, and then I ran into this problem. The symbol I make in create_unamed_typesym has FOwner nil so calling TFPHashObject.ChangeOwner crashes. Did I not make the symbol correctly or do I need to do something before calling ChangeOwner?

Regards,
Ryan Joseph

Ah, you need to use ChangeOwnerAndName then and simply pass in the same name you used for the constructor (cause otherwise it tries to use the name that is currently stored in the list).

Regards,
Sven

On May 13, 2021, at 2:38 PM, Sven Barth wrote: Ah, you need to use ChangeOwnerAndName then and simply pass in the same name you used for the constructor (cause otherwise it tries to use the name that is currently stored in the list).

Looking at this again today and I have yet another question to confirm. I create one of the types using ctypesym.create but the others were just references from the system unit. We only want to change owner of the symbol I create, right? Not the system unit ones? If not changing owner maybe we need to add some ref count or something? just confirming to make sure.

function create_unamed_typesym(def:tdef): tsym;
        var
          newtype: tsym;
        begin
          newtype:=nil;
          if is_conststring_array(def) then
            begin
              { for constant strings we need to respect various modeswitches }
              if (cs_refcountedstrings in current_settings.localswitches) then
                begin
                  if m_default_unicodestring in current_settings.modeswitches then
                    newtype:=cunicodestringtype.typesym
                  else
                    newtype:=cansistringtype.typesym;
                end
              else
                newtype:=cshortstringtype.typesym;
            end
          else if def.typ=stringdef then
            newtype:=tstringdef(def).get_default_string_type.typesym
          else
            begin
              newtype:=ctypesym.create(def.typename,def);
              newtype.owner:=def.owner;
            end;
          if newtype=nil then
            internalerror(2021020904);
          result:=newtype;
        end;

Regards,
Ryan Joseph

On May 15, 2021, at 10:27 AM, Ryan Joseph wrote: Looking at this again today and I have yet another question to confirm. I create one of the types using ctypesym.create but the others were just references from the system unit. We only want to change owner of the symbol I create, right? Not the system unit ones? If not changing owner maybe we need to add some ref count or something? just confirming to make sure.

Also it looks like ChangeOwnerAndName isn't making the compiler happy.

The dynamic array creates an unnamed typesym like this. (def is the array def for [1,2,3])

              newtype:=ctypesym.create(def.typename,def);
              newtype.owner:=def.owner;

And specialized like this:

generic procedure DoThis<T>(p1: T);
begin
end;
begin
  DoThis([1,2,3]);
end;

Then during assembling I get these errors.

Assembling (pipe) /Users/ryanjoseph/Developer/Projects/FPC/macro_test/output/timpfuncspez0.s <stdin>:40:50: error: unexpected token in directive .globl _P$TIMPFUNCSPEZ19_$$_DOTHIS$1$CRCC2373297${Array Of Const/Constant Open} Array of ShortInt ^
<stdin>:41:60: error: unexpected token in directive .private_extern _P$TIMPFUNCSPEZ19_$$_DOTHIS$1$CRCC2373297${Array Of Const/Constant Open} Array of ShortInt ^
<stdin>:42:44: error: unknown token in expression
_P$TIMPFUNCSPEZ19_$$_DOTHIS$1$CRCC2373297${Array Of Const/Constant Open} Array of ShortInt: ^
<stdin>:117:50: error: invalid register name
call _P$TIMPFUNCSPEZ19_$$_DOTHIS$1$CRCC2373297${Array Of Const/Constant Open} Array of ShortInt ^~~~~
<stdin>:117:50: error: Expected an op-mask register at this point call _P$TIMPFUNCSPEZ19_$$_DOTHIS$1$CRCC2373297${Array Of Const/Constant Open} Array of ShortInt ^
<stdin>:283:50: error: unexpected token in '.quad' directive .quad _P$TIMPFUNCSPEZ19_$$_DOTHIS$1$CRCC2373297${Array Of Const/Constant Open} Array of ShortInt ^
error: There were 1 errors compiling module, stopping error: Compilation aborted

Regards,
Ryan Joseph

On May 15, 2021, at 10:49 AM, Ryan Joseph wrote: Also it looks like ChangeOwnerAndName isn't making the compiler happy.

Sorry for the noise, I figured out it was because the name had spaces. How should I make the name then? I'm doing this for now which is certainly unique but it's not standard.

              newtype:=ctypesym.create(def.fullownerhierarchyname(false)+typName[def.typ]+'$'+def.unique_id_str,def);

It will make names like below which don't upset the assembler.

SOMETESTUNIT.arraydef$00005

Anyways, I put up a patch on the bug tracker (https://bugs.freepascal.org/view.php?id=35261) for you to review and see if I did this right.

Regards,
Ryan Joseph

You only need to change the owner of those you create using ctypesym.create. The others are preexisting and you must not touch them in any way.

Regards,
Sven

I had already wondered why that worked previously. But yes, that approach is better.

Regards,
Sven