Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

error recovery after missing semicolon #1025

Closed
davidcarlisle opened this issue Jun 17, 2021 · 10 comments · Fixed by #1026
Closed

error recovery after missing semicolon #1025

davidcarlisle opened this issue Jun 17, 2021 · 10 comments · Fixed by #1026

Comments

@davidcarlisle
Copy link
Member

davidcarlisle commented Jun 17, 2021

Brief outline of the proposed feature

If the user omits a semicolon then tikz issues a quite reasonable error

! Package tikz Error: Giving up on this path. Did you forget a semicolon?.

But if you scroll past that it often ends up in a tight infinite loop so manages to run forever without filling any stack and stopping.

This is particularly troubling with systems like latexmk (and its deployment in Overleaf) as the user never sees the terminal and as the job is killed via a timout the log never gets written properly and the user is never shown the error.

I suggest below making a hard stop after the error message, this works for the Overleaf use case but maybe a bit agressive but perhaps after the error it would be possible to do a quick test if you were in a probable loop and only using \@@end in that case. But otherwise I think always doing a hard stop is safer than looping.

I give a possible code change below but you can see the effect this has on the overleaf UI

current timeout message:

https://www.overleaf.com/read/jwbvcmsgxhzq

Modified example showing missing semicolon error

https://www.overleaf.com/read/zxzfhxjspvcq

\documentclass{article}

\usepackage{tikz}
\makeatletter
\def\tikz@expand{%
  \advance\tikz@expandcount by -1
  \ifnum\tikz@expandcount<0\relax%
    \tikzerror{Giving up on this path. Did you forget a semicolon?}%
%    \let\pgfutil@next=\tikz@finish%
    \def\pgfutil@next{\@@end}%
  \else%
    \let\pgfutil@next=\tikz@@expand
  \fi%
  \pgfutil@next}%
\makeatother

\begin{document}

\begin{tikzpicture}
\node at (0,0) {A}
\node at (1,1) {B};
\end{tikzpicture}
 
\end{document}
@davidcarlisle
Copy link
Member Author

\@@end would need to be defined also for plain or some other suitable form used.

@muzimuzhi
Copy link
Member

I almost opened the same issue two days ago. Perhaps some smoother recovery, instead of a direct \@@end, can be made for missing semicolon between paths. For example, if some common internal of \path-like commands are scanned, insert a semicolon.

@PhelypeOleinik
Copy link
Member

If a hard-end is chosen, I'd recommend \batchmode \read -1 to \x instead of \@@end because TeX doesn't let you \@@end inside a box, for example, so wrapping David's example in a figure environment (a rather common usage), would cause it to loop regardless.

Nevertheless, I think a hard-end is a bit harsh: just aborting the picture sounds more sensible (if that's reasonably doable, of course).

@hmenke
Copy link
Member

hmenke commented Jun 17, 2021

Well, giving up on the path is what is already happening. The problem is that it is not possible for TikZ to detect whether giving up would cause further errors, which is usually why it starts looping.

@ilayn
Copy link
Member

ilayn commented Jun 17, 2021

TeX has the 100 counter for loop detection, and if I remember correctly LaTeX has \maxdeadcycle or something like that. Wouldn't that be possible here for every spin?

@hmenke
Copy link
Member

hmenke commented Jun 17, 2021

\maxdeadcycles is for the output routine. The problem with TikZ is that it loops through expansions.

I think we could already fix some of these problems by making \path eTeX-protected.

@hmenke
Copy link
Member

hmenke commented Jun 17, 2021

I found the problem. When TikZ gives up on the path it does not reinsert the token that it has scanned. In case of \path the macro starts with

\let\tikz@signal@path=\tikz@signal@path% for detection at begin of matrix cell

During expansion TikZ absorbs \let but doesn't reinsert it which leads to \tikz@signal@path being expanded, which is an infinite loop on purpose. Now I only have to find the place where the \let has to be reinserted.

@hmenke
Copy link
Member

hmenke commented Jun 17, 2021

Okay, found the place.

--- a/tex/generic/pgf/frontendlayer/tikz/tikz.code.tex
+++ b/tex/generic/pgf/frontendlayer/tikz/tikz.code.tex
@@ -2436,6 +2436,7 @@
   \endgroup%
   \global\pgflinewidth=\tikzscope@linewidth%
   \tikz@path@do@at@end%
+  \pgfutil@ifnextchar\tikz@signal@path{\let}{}%
 }%
 \let\tikz@lib@scope@check\pgfutil@empty% this is a hook for the scopes library
 \def\tikz@path@do@at@end{\tikz@lib@scope@check}%

Now I only have to find how to reinsert the previously absorbed token and how to do that only on error.

@hmenke
Copy link
Member

hmenke commented Jun 17, 2021

This should now completely sidestep the looping problem and possibly other problems as well. Not happy about the \global though.

--- a/tex/generic/pgf/frontendlayer/tikz/tikz.code.tex
+++ b/tex/generic/pgf/frontendlayer/tikz/tikz.code.tex
@@ -2436,6 +2436,9 @@
   \endgroup%
   \global\pgflinewidth=\tikzscope@linewidth%
   \tikz@path@do@at@end%
+  \expandafter\let\expandafter\tikz@expand@last@token@\csname tikz@expand@last@token\endcsname
+  \global\let\tikz@expand@last@token=\relax
+  \tikz@expand@last@token@
 }%
 \let\tikz@lib@scope@check\pgfutil@empty% this is a hook for the scopes library
 \def\tikz@path@do@at@end{\tikz@lib@scope@check}%
@@ -2486,6 +2489,7 @@
   \advance\tikz@expandcount by -1
   \ifnum\tikz@expandcount<0\relax%
     \tikzerror{Giving up on this path. Did you forget a semicolon?}%
+    \global\let\tikz@expand@last@token=\pgf@let@token
     \let\pgfutil@next=\tikz@finish%
   \else%
     \let\pgfutil@next=\tikz@@expand

@davidcarlisle
Copy link
Member Author

looks good and a lot better than my suggestion of stopping thanks

muzimuzhi added a commit to muzimuzhi/pgf that referenced this issue Jun 17, 2021
muzimuzhi added a commit to muzimuzhi/pgf that referenced this issue Jun 17, 2021
muzimuzhi added a commit to muzimuzhi/pgf that referenced this issue Jun 18, 2021
Co-authored-by: Henri Menke <henri@henrimenke.de>
muzimuzhi added a commit to muzimuzhi/pgf that referenced this issue Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

5 participants