Optimise creation of document commands with m-type args

[josephwright]

READ ME FIRST: Please understand that in most cases we will not be able to merge a pull request because there are a lot of internal activities needed when updating the LaTeX2e sources. If you have a code suggestion please discuss it with the team first.

Pull requests in this repository are intended for LaTeX Team members only.

Internal housekeeping

Status of pull request

• Feedback wanted
• Under development
• Ready to merge

Checklist of required changes before merge will be approved

• Test file(s) added
• Version and date string updated in changed source files
• Relevant \changes entries in source included
• Relevant changes.txt updated
• Rollback provided (if necessary)?
• ltnewsX.tex (and/or latexchanges.tex) updated

[josephwright]

No roll-back - I don't think we need it here

[josephwright]

Not sure if we want to keep the one-step indirection? That would still mark these as 'ltcmd-generated'?

[PhelypeOleinik]

Not sure if we want to keep the one-step indirection? That would still mark these as 'ltcmd-generated'?

I'd rather keep that, because then for other systems like ltcmdhooks we still know what generated the command and we can have some certainty when dealing with them. Removing that indirection turns all into guesswork. It isn't that much slower, is it?

[FrankMittelbach]

looks basically fine to me (assuming that it doesn't break any of the \ShowCommand.. \CopyCommand... thingies, but I can't see how nd the suite should tell us.

So other than perhaps a bit more words in ltnews and also in usrguide I guess (stating that one can use \NewExpanableDocumentCommand instead of \newcommand for consistency and get the same "speed" or something like that) it looks fine

[FrankMittelbach]

Not sure if we want to keep the one-step indirection? That would still mark these as 'ltcmd-generated'?

I'd rather keep that, because then for other systems like ltcmdhooks we still know what generated the command and we can have some certainty when dealing with them. Removing that indirection turns all into guesswork. It isn't that much slower, is it?

I was wondering about that, but what do you gain from ltcmdhooks know that it was done on top-level with \NewExpandableDocumentCommand when all that is there in the end is \def\foo#1#2#3{...}. For adding hooks or showing the definition it seems fairly unimportant if it was produced by hand, by \newcommand or by \NewExpandableDocumentCommand, or not? (unless in the "show" you want to explicitly say how it was produced)

[PhelypeOleinik]

For adding hooks or showing the definition it seems fairly unimportant if it was produced by hand, by \newcommand or by \NewExpandableDocumentCommand, or not?

Not :-)

When the code knows that the command \foo was defined with \NewExpandableDocumentCommand it can patch the command by expanding it with something along the lines of \expandafter\definecmd\expandafter{\foo{##1}{##2}{##3}}, because it knows there are no "weird" tokens in the parameter text. If we don't know that, the code has to resort to patching with \scantokens which is... sketchy.

[FrankMittelbach]

Not :-)

ok point taken but it should be a really light indirections that is basically a single extra expansion

[josephwright]

Not sure if we want to keep the one-step indirection? That would still mark these as 'ltcmd-generated'?

I'd rather keep that, because then for other systems like ltcmdhooks we still know what generated the command and we can have some certainty when dealing with them. Removing that indirection turns all into guesswork. It isn't that much slower, is it?

All doable. I am then wondering about \__kernel_cmd_if_xparse:NTF. First, shouldn't it be \__kernel_cmd_if_ltcmd:NTF? More importantly, why does it do all of these tests:

\cs_new_protected:Npn \@@_cmd_type_cases:NnnnnF #1 #2 #3 #4 #5 #6
  {
    \exp_args:Ne \str_case_e:nnF
      {
        \exp_args:Nf \tl_if_empty:nT { \cs_argument_spec:N #1 }
          { \exp_not:N \exp_not:n { \exp_not:e { \tl_head:N #1 } } }
      }
      {
        { \exp_not:N \@@_start:nNNnnn } {#2}
        { \exp_not:N \@@_start_expandable:nNNNNn } {#3}
        { \exp_not:N \@@_start_env:nnnnn } {#4}
        {
          \exp_after:wN \exp_not:N
            \cs:w environment~
              \exp_last_unbraced:Ne \use_none:nnn
                { \cs_to_str:N #1 } ~end~aux \cs_end:
        } {#5}
      }
      {#6}
  }

rather than simply check if \foo code exists? I'll need to adjust to allow for optimisation, so before I do can we agree on what this test should be?

[josephwright]

Also similar in \@@_copy:NN, which again seems to test for the internal setup (even though the test happens before it's called too).

[josephwright]

So other than perhaps a bit more words in ltnews and also in usrguide I guess (stating that one can use \NewExpanableDocumentCommand instead of \newcommand for consistency and get the same "speed" or something like that) it looks fine

That's a bit more tricky due to \long status, etc.

[josephwright]

I've added one level of indirection. Note that this means that an error shows \foo code rather than \foo or \foo[space]: is that OK? We could fix with two levels of indirection, but I wonder if that is a bit much.

[josephwright]

I am then wondering about \__kernel_cmd_if_xparse:NTF. First, shouldn't it be \__kernel_cmd_if_ltcmd:NTF? More importantly, why does it do all of these tests:

We might need to test for end-of-env, but all of the others have \foo code, no?

[josephwright]

I am then wondering about \__kernel_cmd_if_xparse:NTF. First, shouldn't it be \__kernel_cmd_if_ltcmd:NTF? More importantly, why does it do all of these tests:

We might need to test for end-of-env, but all of the others have \foo code, no?

OK, I see that for copying there are different paths (so we will need to extend), but does this need to share the code with the yes/no test?

[josephwright]

I need to adjust to skip optimisation for envs: currently doing that gives some very odd situation with building source2e. I need to track this down - may take a bit.

[PhelypeOleinik]

rather than simply check if \foo code exists?

The problem in this case is if someone does \def\bar{something} \let\foo\bar, then \foo code exists, but \foo is not an ltcmd command. This case is particularly problematic for \@@_show:N and \@@_copy:NN, which rely heavily on the internal structure of the command being correct.

[FrankMittelbach]

why not simply \protected\def\foo{\flag\foocode} with \let\flag\@empty(and a suitable internal name)?

[josephwright]

why not simply \protected\def\foo{\flag\foocode} with \let\flag\@empty(and a suitable internal name)?

That had also occurred to me: I was initially aiming for a simple structure, but a flag might serve as well as anything else.

[car222222]

Not to be confused with an l3 flag:-).

[muzimuzhi]

Ltnews adjustments for reintroduced redirection and possibility to optimise +m-arg only cases.

[muzimuzhi]

rather than simply check if \foo code exists?

The problem in this case is if someone does \def\bar{something} \let\foo\bar, then \foo code exists, but \foo is not an ltcmd command.

Promoting \NewCommandCopy against \let to users is a long-term task...

[PhelypeOleinik]

Promoting \NewCommandCopy against \let to users is a long-term task...

Agreed, but in this case \NewCommandCopy wouldn't help, as \bar was defined with \def (or \newcommand, for that matter), so using \NewCommandCopy here would have the same effect as using \let plus some overhead processing.

[Skillmon]

Not :-)

ok point taken but it should be a really light indirections that is basically a single extra expansion

Just \cs_new_eq:NN \__ltcmd_defined: \prg_do_nothing: and putting \__ltcmd_defined: at the start of the \meaning should be fine and no big overhead. (so without the \foocode step).

[josephwright]

Not :-)

ok point taken but it should be a really light indirections that is basically a single extra expansion

Just \cs_new_eq:NN \__ltcmd_defined: \prg_do_nothing: and putting \__ltcmd_defined: at the start of the \meaning should be fine and no big overhead. (so without the \foocode step).

More or less what I've done.

[muzimuzhi]

I just find what I was looking for yesterday:

Now \group_align_safe_begin: and \group_align_safe_end: are dropped in optimized commands. \conditionally@traceoff and \conditionally@traceon are dropped in optimized non-expandable commands in addition.

If that's the decision, this trade off may need some words in documentation.

[FrankMittelbach]

Now \group_align_safe_begin: and \group_align_safe_end: are dropped in optimized commands. \conditionally@traceoff and \conditionally@traceon are dropped in optimized non-expandable commands in addition.

I missed that, where are they dropped?

[muzimuzhi]

@FrankMittelbach I worried too much. Optimized commands need neither \group_align_safe_(begin|end): and \conditionally@trace(off|on).

[Skillmon]

@muzimuzhi it has the potential to change behaviour (check with the current behaviour):

\documentclass{article}

\NewDocumentCommand\mygobble { m } {}
\newcommand\gobble[1]{}

\begin{document}
\begin{tabular}{ll}
  a\mygobble&&b \\ % fine
  c\gobble&&d % error
\end{tabular}
\end{document}

[josephwright]

@muzimuzhi it has the potential to change behaviour (check with the current behaviour):

\documentclass{article}

\NewDocumentCommand\mygobble { m } {}
\newcommand\gobble[1]{}

\begin{document}
\begin{tabular}{ll}
  a\mygobble&&b \\ % fine
  c\gobble&&d % error
\end{tabular}
\end{document}

That’s protected status , which is unchanged

[Skillmon]

That’s protected status , which is unchanged

The same behaviour change would occur with \NewExpandableCommand, currently it uses \group_align_safe_... so would work in above MWE, with the shortcut behaviour dropping \group_align_safe_... this would change (though unlikely to have any real effect in the field, it's a behaviour change that I wanted to make sure was noted).

[FrankMittelbach]

this would change (though unlikely to have any real effect in the field, it's a behaviour change that I wanted to make sure was noted).

I think a comment on that difference next to the code definition might be sensible, but I wouldn't go further and document it on the level of the user documentation because being able able to grab and destroy a & is not really an intended function for such command

[josephwright]

this would change (though unlikely to have any real effect in the field, it's a behaviour change that I wanted to make sure was noted).

I think a comment on that difference next to the code definition might be sensible, but I wouldn't go further and document it on the level of the user documentation because being able able to grab and destroy a & is not really an intended function for such command

Done