Intarray #2151

brucemiller · 2023-07-14T01:11:52Z

This PR provides the intarray infrastructure needed by the latest (2023) TeXLive versions of expl3. It no longer fails with errors, but still takes way too long to run (working on that).

This partially, but not completely, addresses #2044 and #2064.

…red font metric data, since 2023's expl3 code is using it for intarray storage

…expl3

…xtensions (in Perl)

dginev · 2023-07-14T02:05:10Z

I would also like to invite @teepeemm to offer a review here (if he's willing and interested), given that he did very relevant overlapping work in #2109 . No obligation to do so, but he's welcome to!

dginev

Looks good on a surface read! I left some low-level perl comments.

dginev · 2023-07-14T14:00:42Z

lib/LaTeXML/Package/expl3.lua.ltxml

+    my ($stomach, $pos, $table, $value, $ifmissing) = @_;
+    my $length = scalar(@$table);
+    $pos = $pos->valueOf;
+    if (($pos > 0) && ($pos < $length)) {


Is there an off-by-one error here, or am I misreading the bounds?

Say, if $table = [SomeSingleElement]; and $pos is 1, then $length is 1 as well, but $pos == $length. I think we won't be able to assign an element in that slot?

So maybe $pos < $length should be $pos <= $length ?

There are a number of other subs with this $$table[$pos - 1] kind of indexing, so posing the question in general for all of them.

I think it might be easier to use 1-indexed arrays throughout the file (along with the comment to that affect). We'd need scalar(@$table)-1 or $#table when we need the length, and I don't think $table[0] being undef would cause problems.

But all that wasted space!?!?! I think I'll be better off keeping the Perl arrays 0-based, but do need to be careful about the limits. Patch coming.

dginev · 2023-07-14T14:10:06Z

lib/LaTeXML/Package/TeX.pool.ltxml

+    my $f = ($at ? $at->divide(Dimension('1em'))->valueOf
+      : ($scaled ? $scaled->valueOf / 1000
+        : 1));
+    my $fontinfo = {%props};


Out of curiosity, why not my $fontinfo = \%props; ?
I think that avoids cloning the hash, and if I'm reading the code right %props has no further uses that may cause data race conflicts.

lib/LaTeXML/Package/TeX.pool.ltxml

dginev · 2023-07-14T14:13:54Z

lib/LaTeXML/Package/expl3.lua.ltxml

+    my $tok = $gullet->readXToken;
+    if (my $defn = LookupDefinition($tok)) {    # token may be \let to the identifier!!!
+      $tok = $defn->getCS; }
+    if ($tok && $tok->equals(T_CS('\__intarray:w'))) {


Interesting, the 3 lines here seem to be a good possible use of the new $tok->defined_as(T_CS('\__intarray:w')), right?

The current code looks correct too, just marking the opportunity.

dginev · 2023-07-14T14:17:59Z

lib/LaTeXML/Package/expl3.lua.ltxml

+DefMacroI('\__intarray_range_to_clist:w', 'Intarray Number Number', sub {
+    my ($stomach, $table, $from, $to) = @_;
+    $from = $from->valueOf - 1;
+    $to   = $to->valueOf - 1;


This is a rare edge case, but if either of the input numbers happens to be (erroneously?) zero 0, the $from or $to index will be -1, which will lead to a rather peculiar indexing in perl. It may be worth guarding them to be within the allowable range of $table here as well.

It seems like the expl3 code usually has the public functions do the input validation, and then the __private functions don't need to worry about that. The corresponding lua code doesn't validate the inputs. (I've also not noticed a public version of this function, so it might all be moot.)

teepeemm · 2023-07-19T16:37:37Z

lib/LaTeXML/Package/expl3.lua.ltxml

+    return; },
+  protected => 1);
+
+DefPrimitiveI('\intarray_gzero:w', 'Intarray', sub {


My dtx file is defining \intarray_gzero:N at this point, and no \intarray_gzero:w.

I wasn't completely clear on the lua interface, so I just implemented (or didn't) everything related to intarray.

teepeemm · 2023-07-19T16:42:42Z

lib/LaTeXML/Package/expl3.lua.ltxml

+    $to   = $to->valueOf - 1;
+    return Tokenize(join(', ', @$table[$from .. $to])); });
+
+DefPrimitiveI('\__intarray_gset_range:w', 'Number Intarray', sub {


Since \__kernel_intarray_gset_range_from_clist:Nnn calls \__intarray_gset_range:w \int_eval:w #2 #1 #3 , , \scan_stop:, I think this will leave #3 , , \scan_stop: in the token stream. Should this be something like

DefPrimitiveI('\__intarray_gset_range:w', LaTeXML::Core::Parameters->new( LaTeXML::Core::Parameter->new('Number'), LaTeXML::Core::Parameter->new('Intarray'), LaTeXML::Core::Parameter->new('Until','Until:\scan_stop:',extra=>[T_CS('\scan_stop:')]) ), sub { ... } );

On the other hand, there doesn't appear to be a public version of the function, so I'm not sure how often this will get called (also, being unimplemented will throw an error anyway).

teepeemm · 2023-07-19T19:01:46Z

lib/LaTeXML/Package/expl3.lua.ltxml

+      $tok = $defn->getCS; }
+    if ($tok && $tok->equals(T_CS('\__intarray:w'))) {
+      my $n       = $gullet->readNumber;
+      my $address = '__intarray' . '_' . ToString($tok) . '_' . $n->valueOf;


Not really a bug, but having every $address begin with __intarray_\__intarray:w_ seems a bit redundant. (Unless there's some other way to get an intarray that I wasn't finding.)

Honestly, I wasn't completely clear on exactly how these were being marked from the TeX end (or lua end), so I went with redundant just in case.

…iven file if (& before) it is loaded

@expl

…r with expl3's \@expl@push@filename@aux@@ dangerous way of knowing the file that is about to be loaded

dginev

Looks good!

brucemiller · 2023-07-19T20:54:20Z

I think this one's ready, if you guys agree.

teepeemm · 2023-07-19T21:52:32Z

We do still need \intarray_gzero:N, which can use the :w definition. I'm moderately certain that we don't need \intarray_gzero:w.

I'm still trying to dig into why it takes so long to go through UnicodeData, but that shouldn't stop this from merging.

brucemiller · 2023-07-20T02:22:31Z

Thanks for the reviews. I still would like to come back later to #2109 to see if we can speed things up, but given the evolving nature of latex3, I'd want to avoid building in too much expl3 code (even translated into perl); it'll likely be different tomorrow.

brucemiller added 3 commits July 13, 2023 19:17

Have \font, \fontdimen, \hyphenchar,\skewchar actually connect to sto…

55cfd9e

…red font metric data, since 2023's expl3 code is using it for intarray storage

New expl3.lua for Perl analogues of (a subset of) lua extensions for …

e47e475

…expl3

Cleanup binding only loading the style file, but preloading the lua e…

3f70b82

…xtensions (in Perl)

brucemiller requested a review from dginev July 14, 2023 01:11

dginev reviewed Jul 14, 2023

View reviewed changes

teepeemm reviewed Jul 19, 2023

View reviewed changes

brucemiller added 4 commits July 19, 2023 16:02

Refer to fontinfo hash, rather than copy it

dd39822

Use ->defined_as, and proper array bounds checking, per Deyan Ginev

7f87134

A more robust AutoLoad capability that undefines all triggers for a g…

ce48d3f

…iven file if (& before) it is loaded

A safer approach to \@pushfilename book-keeping that cooperates bette…

78f0509

…r with expl3's \@expl@push@filename@aux@@ dangerous way of knowing the file that is about to be loaded

dginev approved these changes Jul 19, 2023

View reviewed changes

Fix typo; thanks teepeemm

d35b35e

brucemiller merged commit 202aced into master Jul 20, 2023

brucemiller deleted the intarray branch July 20, 2023 02:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Intarray #2151

Intarray #2151

brucemiller commented Jul 14, 2023

dginev commented Jul 14, 2023

dginev left a comment

dginev Jul 14, 2023

teepeemm Jul 19, 2023

brucemiller Jul 19, 2023

dginev Jul 14, 2023

dginev Jul 14, 2023

dginev Jul 14, 2023

teepeemm Jul 19, 2023

brucemiller Jul 19, 2023

teepeemm Jul 19, 2023

brucemiller Jul 19, 2023

teepeemm Jul 19, 2023

teepeemm Jul 19, 2023

brucemiller Jul 19, 2023

dginev left a comment

brucemiller commented Jul 19, 2023

teepeemm commented Jul 19, 2023

brucemiller commented Jul 20, 2023

Intarray #2151

Intarray #2151

Conversation

brucemiller commented Jul 14, 2023

dginev commented Jul 14, 2023

dginev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dginev left a comment

Choose a reason for hiding this comment

brucemiller commented Jul 19, 2023

teepeemm commented Jul 19, 2023

brucemiller commented Jul 20, 2023