[fpc-pascal] Constants in generics

[Sven Barth]

Am Mo., 26. Nov. 2018, 10:46 hat Ryan Joseph <ryan at thealchemistguild.com>
geschrieben:

>
>
> > 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.
>

Yes, but that is only because the code currently only handles types. For
constants the addition of the type is not required, only the constant
value.


> > - 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.
>

Those assumptions will "simply" have to be checked/fixed. In the end we'll
have cleaner code and thus it will be worth it. And with the help of the
testsuite you can avoid regressions. :)


> > - 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.
>

Maybe you can show a test case that is failing?


> > - 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.
>

For now we should reject an inline specialization (aka block_type =
bt_general) in mode Delphi if the found generic contains any constant
parameter (declarations of such generics should be allowed). Later on we
can lift that restriction once I've implemented the checks I mentioned.


> >
> > 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.
>

As I said above that code will have to become aware of constants. Might
lead to more crashes for you right now, but the end result would be
cleaner/better. As a help you could compile the compiler with -CR which
would ensure that typecasts of classes are always done with "as", thus
triggering an exception at the correct location.


> >
> > 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,
Sven

>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freepascal.org/pipermail/fpc-pascal/attachments/20181126/62b46c08/attachment.html>