[fpc-pascal] Constants in generics

[Ryan Joseph]

> On Nov 26, 2018, at 12:09 AM, Sven Barth via fpc-pascal <fpc-pascal at lists.freepascal.org> wrote:
> 
> - your pretty name is wrong; the pretty name for a specialization with constants should be "TSomeGeneric<TSomeType, 42>", not "TSomeGeneric<SomeUnit.TSomeType, System.LongInt#42>" as it would be now

I’ll change the # prefix for const params but the unit prefix was not added by me. See the line:

prettynamepart:=typeparam.resultdef.fullownerhierarchyname(true);

in pgenutil.pas.

> - remove those tgenericparasym and tgeneric_*_parasym types; use simply tconstsym instead of ttypesym for const parameters, other code will have to check that correctly

Ok, I was able to remove those and replace with tconstsym. Much cleaner now. 

> - even though you'll remove is_const and const_type from ttypesym again due to the above it's wrong to use putsmallset/getsmallset for an enum; use putbyte/getbyte or one of the "larger" put*/get* functions depending on the number of entries contained in the enum (Note: not a runtime check, just pick the correct one at compile time)

Are you sure those should be changed? There seems to be lots of assumptions made that the generic params are ttypesym and I’m not confident that mixing in tconstsyms will be safe. Having said that I never liked that I mixed in those const_* members into the class.

Like you mention below if the const type was restricted in the definition then it would make sense to change these to tconstsym. I guess I need to try and see how much code this change blows up.

> - get rid of tscannerfile.parsing_generic_type; it's not only at the wrong location (the name "parsing_generic_type" should already tell you that it has no place inside the *scanner*), but it's also used to solve something in a completely wrong way: you must *not* use current_structdef for the generic parameters as the only valid parameter list inside parse_generic_specialization_types_internal *is* paradeflist and genericdef.genericparas. If there is still a problem then you need to handle that differently (Note: also don't Exit from parse_generic_specialization_type_internal, because the idea is that all incompatibilities are shown at once, so use Continue to check the next parameter)

Yes, that was a hack to access current_structdef safely but I guess that’s not what I should be doing. Let me see if I can get that same information from the 2 params you mentioned.

> - remove the check for m_objfpc; yes it won't work with inline specializations in mode Delphi for now, but it can still be used for specializations in type or var sections; also one idea for an improvement I have for parsing inline specializations in mode Delphi would be if the compiler knows that the symbol left of the "<" can only be a generic (cause let's be honest: in most code types aren't overloaded with variables/constants and more often than not types begin with "T", so they shouldn't conflict with the names of more local variables/constants/fields either)

I got a crash in Delphi mode but I can change that to an error.

> 
> Also thinking about the feature a bit it would be better to enforce the type of the constant during the generic's declaration. After all I can do different things with a string const, an ordinal const or a set const. So it would be better to use "const <NAME>: <TYPE>" where type can be any type that results in a tconstsym. After all the type of the constant inside the generic is usually fixed and those few cases were a variadic constant would be necessary could be handled by "TMyGeneric<T; const N: T>" (which would not work right now, but as we also need to support "TMyGeneric<T; S: SomeIntf<T>>" due to Delphi compatibility that would be handled by the same solution).

> This way you can also get rid of the cconstundefined and you can create a correct tconstsym right of the bat.

Sure but I need to see how many assumptions this undoes in other areas. I seem to remember having problems getting the const to fit in as it was and that’s when they were all the same class.

> 
> And as I had written for the helper extension: add tests. A feature like this will need many tests (you can use "tgenconst" as test name prefix). Also don't forget to test this with generic routines.
> 
> Don't forget to check the code formatting as well, I saw a few locations where you had spaces where they don't belong. ;)

Regards,
	Ryan Joseph