\dots fails if followed by commands involving primitive `\if`
Opened this issue · 13 comments
Brief outline of the bug
The code added at #1424 to expand past a \protect definition may expose a primitive \if... token
this reacts badly when nested in the testing done by \dots.
Minimal example showing the bug
\RequirePackage{latexbug} % <--should be always the first line (see CONTRIBUTING)!
\documentclass{article}
\usepackage{amsmath}
\newcommand\test{\iftrue x\fi}
\DeclareRobustCommand\testr{\DOTSB+}
\protected\def\testp{\DOTSB-}
\def\testd{\DOTSB*}
\begin{document}
$ x \dots \testr$
$ x \dots \testp$
$ x \dots \testd$
$x \dots \iffalse a\else b\fi $
$x \dots \test$
$x \dots \boldsymbol{y} $
\end{document}
gives
Runaway argument?
\fi \fi \thedots@ \iffalse a\else b\fi $
! Paragraph ended before \stripprotect@ was complete.
<to be read again>
\par
l.21
I think the best solution in the end would be to f-expand before testing so that primitve \ifs, \protect and \protected and \long definitions would all be resolved in advance and the testing simplified, however it would be a disruptive change as the current \DOTSI, \DOTSB, .. definitions wouldn't be distinguishable (as they are all \ifx equal to \relax).
So the suggestion here is more in the spririt of the existing code, check after the first \meaning if the token is a primitive \if.. (because meaning starts \if) and if so skip the test that expands and checks for \protect.
*** amsmath.sty~ Sun Aug 25 14:24:19 2024
--- amsmath.sty Sun Aug 25 14:44:14 2024
***************
*** 443,448 ****
--- 443,452 ----
\@xp\Umathch@\meaning@\Umathch@
}
\fi
+ {\uccode`7=`\\ \uccode`(=`i \uccode`)=`f
+ \uppercase{\gdef\testif@#1#2#3#4\testif@{%
+ \ifx7#1\ifx(#2\ifx)#3\@tempswafalse\fi\fi\fi}
+ }}
\newcount\classnum@
\def\getmathch@#1.#2\getmathch@{\classnum@#1 \divide\classnum@4096
\ifcase\number\classnum@\or\or\gdef\thedots@{\dotsb@}\or
***************
*** 522,531 ****
\ifgtest@ % if \keybin@ test
\gdef\thedots@{\dotsb@}%
\else
\begingroup
\def\protect{\protect}% % make it a quark
\xdef\meaning@{\@xp\stripprotect@\@let@token.........\stripprotect@. .........}%
! \endgroup
\xdef\meaning@@{\@xp\striplong@\meaning@\relax\meaning@}%
\xdef\meaning@@{\@xp\stripprotected@\meaning@@\relax\meaning@@}%
\@xp\math@\meaning@\math@
--- 526,540 ----
\ifgtest@ % if \keybin@ test
\gdef\thedots@{\dotsb@}%
\else
+ \xdef\meaning@{\meaning\@let@token. .........}%
+ \@tempswatrue
+ \@xp\testif@\meaning@....\testif@
+ \if@tempswa
\begingroup
\def\protect{\protect}% % make it a quark
\xdef\meaning@{\@xp\stripprotect@\@let@token.........\stripprotect@. .........}%
! \endgroup
! \fi
\xdef\meaning@@{\@xp\striplong@\meaning@\relax\meaning@}%
\xdef\meaning@@{\@xp\stripprotected@\meaning@@\relax\meaning@@}%
\@xp\math@\meaning@\math@
Full .sty attached for ease of testing. (I could make a PR against the dtx source later, but possibly not this weekend)
The fix above was released in the 2024-11-01 release but unfortunately misses one case using \protected in combination with a primitive \if.
An extended test file with an extra case (\testpi) that fails:
\documentclass{article}
\usepackage{amsmath}
\newcommand\test{\iftrue x\fi}
\protected\def\testpi{\iftrue x\fi}
\DeclareRobustCommand\testr{\DOTSB+}
\protected\def\testp{\DOTSB-}
\def\testd{\DOTSB*}
\begin{document}
$ x \dots \testr$
$ x \dots \testp$
$ x \dots \testd$
$x \dots \iffalse a\else b\fi $
$x \dots \test$
$x \dots \boldsymbol{y} $
$ x \dots \testpi$
\end{document}
It remains to be seen whether to re-adjust the \meaning tests or to bring forward the code planned for #1521
Possibly related: #1531 (comment)
The same error just occurred to me, when using \dots in a matrix environment.
Extra \fi.
\mdots@@ ...haracter@ \fi \fi \fi \fi \fi \fi \fi
\thedots@
l.61 \var{X_1} & \cov{X_1}{X_2} & \dots &
\cov{X_1}{X_N} \\
The cause was not immediately clear to me, as my custom commands \var and \cov do not include any \if:
\NewDocumentCommand\var{m}{\mathrm{var}\left(#1\right)}
\NewDocumentCommand\cov{mm}{\mathrm{cov}\left(#1,#2\right)}
Apparently, there have to be some nested \ifs. I hope, this will be ressolved with the same fix.
@guger there is an \ifhmode inserted by the array package tabular code, which then fails, which is why it's enough to put anything between \dots and & so these are all the same issue. A fix will be posted shortly, but (especially as the fix that was posted with the release proved incomplete) it needs a bit more testing, but we'll push a fix (or temporarily revert the change) assoon as possible.
@davidcarlisle Very interesting, thank you for the explanation!
@guger after loading array if you add
\makeatletter
\protected\def\textonly@unskip{\relax\ifhmode\unskip\fi}
\makeatother
to your preamble could you confirm the problem goes (this isn't the real fix, which should be in amsmath but this adjusts array not to trigger the problem
@davidcarlisle Yes, this solves the issue! Thanks!
@guger thanks for confirming. That change (adding \relax) is harmless so it would be Ok to leave it in your document even after an update to amsmath is rolled out, but sorry for the inconvenience.
I have observed a related bug -- should I open a new issue about it? The following minimal code
\documentclass{article}
\usepackage{amsmath}
\begin{document}
$A=\{1,2,\dots\}$
\end{document}triggers
! TeX capacity exceeded, sorry [input stack size=10000].
\GenericWarning ...tchoice@ \else 4\fi \endcsname
\protect \GenericWarning
l.5 $A=\{1,2,\dots\}
$
! ==> Fatal error occurred, no output PDF file produced!
(Using \ldots works fine.)
@clason That looks like the same issue. Does the code @davidcarlisle posted above help?
It does not, unfortunately.
@clason yes sorry, that example was posted to https://tex.stackexchange.com/questions/730161/recently-added-bug-to-amsmath yesterday, it's the same issue in amsmath but as it isn't in a table the above array patch doesn't avoid it. There is a quick fix posted at the above site, In these github sources amsmath has been reverted to the June release see #1543 and we'll re-issue amsmath today. We will bring forward new code implementing the intended feature of supporting protected macros after testing with the test cases that have been raised, but we are first reverting the amsmath release to allow time for testing.
Thanks, the tex.se post is indeed exactly the same. (Cue lament on the declining usefulness of Google...)
As the workaround is trivial and harmless (just add an l in those cases), it's not urgent.