[fpc-pascal] Constants in generics

Sven Barth pascaldragon at googlemail.com
Mon Nov 26 11:16:07 CET 2018

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

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

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


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

More information about the fpc-pascal mailing list