Unsorted ref output when "sibling=<counter>" is used with cleveref

[muzimuzhi]

MWE and short description

\documentclass{article}
\usepackage{thmtools}
\usepackage{cleveref}

% share the theorem counter with "subsection"
\declaretheorem[sibling=subsection]{theorem}

\begin{document}

\section{title}
\begin{theorem}
Blah.
\end{theorem}

\begin{theorem}\label{theorem} % thm 2.1
Blah.
\end{theorem}

\section{title}
\begin{theorem}\label{othertheorem} % thm 1.2
Blah.
\end{theorem}

\Cref{theorem,othertheorem}.
\end{document}

Since cleveref sorts the references, we should get "Theorems 1.2 and 2.1", instead of "Theorems 2.1 and 1.2".

More info

From https://tex.stackexchange.com/q/598167. As indicated by Willie Wong's answer, the \newlabel lines written to aux are wrong.

This is reproducible with texlive 2018 on overleaf.com, indicating it was an aged bug existing before I maintained thmtools (have I ever did the work?).

[wwywong]

I dug a little bit more.

If you use cleveref without thmtools or amsthm or ntheorem, and create a theorem using

\newtheorem{theorem}[subsection]{Theorem}

you find in your aux file \newlabel{othertheorem@cref}{{[subsection][1][2]2.1}{[1][1][]1}} which is as expected: because @thm calls \refstepcounter{subsection} in this case using the standard latex.ltx definition.

As soon as you load thmtools, the aux file line becomes \newlabel{othertheorem@cref}{{[theorem][1][]2.1}{[1][1][]1}}. This is interesting since the only way for this to happen is if somewhere instead of \refstepcounter{subsection}, it is now calling \refstepcounter{theorem}; in the standard latex definition and the amsthm definition, this counter should not be defined and things should break.

The reason this output shows up should be because of thm-autoref.sty which maps the counter theorem directly to subsection through the code in aliasctr.sty. The code in thm-autoref.sty is interesting because it explicitly undoes a bunch of stuff in thm-patch.sty: the hook to \thmt@autorefsetup means that when one calls \newtheorem{theorem}[subsection]{Theorem}, what thm-patch.sty passed onto the old definition of \newtheorem is \newtheorem{theorem}[theorem]{Theorem}, which in all cases makes the optional argument functionally redundant. (As an aside this is probably something that should be refactored and cleaned up.)

I think to patch this:

1. One option is to add in the code for \@counteralias in aliasctr.sty an additional check to see if the sibling counter is in the reset list of any other defined counter (xassoccnt has code that does something similar) and add the aliased theorem counter to the same reset list. This way the check by cleveref would work.
2. Another option is to make whether the line \protected@edef\thmt@sibling{\thmt@envname} in thm-autoref.sty is called be dependent on whether ntheorem or amsthm are loaded: if they are, then kill that line; if they aren't then keep that line. This way you will still have autoref working, but when ntheorem or amsthm is loaded cleveref will do the right thing. (But cleveref will still be wrong when ntheorem and amsthm are not loaded. )

[muzimuzhi]

@wwywong I believe you are now more familiar with the thmtools codebase than me. Please give me some more time to read and understand the current implementation, before I could participate in the discussion. So sorry.

[wwywong]

@muzimuzhi no worries! Take your time. I don't actually use thmtools so I have nothing riding on this. :-)

FWIW: a cheap change is that you can just update the user manual to tell users that if they use a sibling counter that is reset by another counter and want to use cleveref, they should manually register the counter in to the reset list.

So for example, if we have

\documentclass{article}

\usepackage{thmtools}
\usepackage{cleveref}

\declaretheorem[sibling=subsection]{theorem}

Since by default subsection is reset by section, one should add

\counterwithin{theorem}{section}

so that the "fake" counter created by thm-autoref.sty shows up in the reset list, and gets recognized by cleveref. This is not the cleanest solution but is an easy-enough-to-implement workaround that is suitable for most users, until the code gets fixed.

[muzimuzhi]

The problem came from aliasctr.sty. Heiko Oberdiek's package aliascnt provides similar utilities, and cleveref had contained compatibility code for it. Since cleveref is not actively maintained since 2018, it seems the similar compatibility problem had to be handled from thmtools' side.

Here is an example using a manually aliased counter.

\documentclass{article}
\usepackage{cleveref}

\makeatletter
% provide counter "foo" which is an alias of "subsection"
\let\c@foo\c@subsection
\let\cl@foo\cl@subsection
\let\thefoo\thesubsection

% helper macro
\def\test{%
  \refstepcounter{foo}\thefoo,
  % debug info
  \texttt{current label: \detokenize\expandafter{\cref@currentlabel}}\par}
\makeatother

% config cleveref
\Crefname{foo}{Foo}{Foos}

\begin{document}
\section{title}
\test\test\label{key:a}

\section{title}
\test\label{key:b}

\Cref{key:a,key:b}
\end{document}

expected: Foos 1.2 and 2.1 actually: Foos 2.1 and 1.2

[muzimuzhi]

The mentioned compatibility code for aliascnt package, in cleveref.dtx, v0.21.4 (online, download), lines 7572--7598.

% \subsubsection{\package{aliascnt} support}
% \begin{macro}{aliascnt}
%   For the \package{aliascnt} trick described in \cref{sec:label_type}
%   of the documentation to work, we have to inform \package{cleveref}
%   about how aliased counters get reset. \package{aliascnt}'s
%   \cmd{\newaliascnt} command doesn't add the aliased counter to any
%   reset list, but if the counter it's aliased to gets reset, the
%   aliased counter will get reset too. In order for \package{cleveref}
%   to correctly sort cross-references to the aliased counter, we have to
%   add that counter to the appropriate reset list, even though that
%   isn't necessary to actually reset the counter itself. We add this to
%   the \cmd{\newaliascnt} command.
% \end{macro}
% \begin{macro}{\newaliascnt}
%    \begin{macrocode}
\@ifpackageloaded{aliascnt}{%
  \PackageInfo{cleveref}{`aliascnt' support loaded}%
  \let\cref@old@newaliascnt\newaliascnt%
  \renewcommand*{\newaliascnt}[2]{%
    \cref@old@newaliascnt{#1}{#2}%
    \cref@resetby{#2}{\cref@result}%
    \ifx\cref@result\relax\else%
      \cref@addtoreset{#1}{\cref@result}%
    \fi}%
  }{}%  end of \@ifpackageloaded{aliascnt}
%    \end{macrocode}
% \end{macro}

Also see the usage in doc of cleveref, sec. 6 and doc of aliascnt. I wonder if the manually inserted line \aliascntresetthe{<cnt>} in the mentioned usages could be auto-completed, for the case in thmtools.

[muzimuzhi]

https://github.com/muzimuzhi/thmtools/commit/4e35ba2f0f31254bda4a0fdf45c7683e6e5d8f4c fixed the simplified example, but not the original one.

[wwywong]

4e35ba2 fixed the simplified example, but not the original one.

That looks promising! Great idea! You are just missing \expandafters (thm-autoref passes things in as a macro) I've just tested with

\expandafter\cref@resetby\expandafter{#2}{\cref@result}

and it works.

[muzimuzhi]

Great observation. To make \@counteralias generally usable, I think something like

\edef\aliasctr@temp{%
  \noexpand\cref@resetby{#2}{\noexpand\cref@result}}%
\aliasctr@temp

added to \@counteralias or the usage of this command, would be better.

Would you like to be (in the sense of git history) a contributor of thmtools? Either opening a pull request or providing your possible no-reply email address so I could co-author you in commits does the job.

[wwywong]

That makes sense, and is indeed better.

My Git no-reply is 19225547+wwywong@users.noreply.github.com

[Jinwen-XU]

Actually the current code is still not always guaranteed to work. Consider the following example:

\documentclass{article}
\usepackage{thmtools}
\usepackage{cleveref}

\newcounter{test}
\newcounter{subtest}
\counterwithin{subtest}{test}

% share the theorem counter with "subsection"
\declaretheorem[sibling=subtest]{theorem}

\begin{document}

\section{title}
\stepcounter{test}

\begin{theorem}
Blah.
\end{theorem}

\begin{theorem}\label{theorem} % thm 2.1
Blah.
\end{theorem}

\section{title}
\stepcounter{test}

\begin{theorem}\label{othertheorem} % thm 1.2
Blah.
\end{theorem}

\Cref{theorem,othertheorem}.
\end{document}

Everything is the same, except here I'm using a custom counter test instead of section. You will still get "Theorems 2.1 and 1.2".

---

Analysis

The problem here is that \cref@resetby is just an internal method to get the parent counter, for example \cref@resetby{subsection}{\macro} gives you section in \macro. However, if one looks into \cref@resetby, one shall find that this command is hard-coded, so it definitely won't work for custom counters. And then, \cref@addtoreset{#1}{\cref@result} is basically

\tl_gput_right:cn { cl@ \cref@result } { \@elt { #1 } }

Thus, in your original example, what actually happened is simply \tl_gput_right:cn { cl@ section } { \@elt { theorem } }.

However, \tl_gput_right:cn { cl@ test } { \@elt { theorem } } does not work in my example, perhaps cleveref uses the hard-coded information somewhere else in the code, so this won't be enough. We shall need to dig deeper into the code of cleveref.

Since cleveref has not been updated for years, I doubt if it is worth the effort. Perhaps it is time to consider switching to zref-clever or something like this, before further attempts being made (in the next decade).

[Jinwen-XU]

Following my last comment. In my previous example, this can be fixed with (provided that regexpatch is loaded and \makeatletter is applied):

\cref@addtoreset{theorem}{test}
\xpatchcmd{\cref@resetby}{\let#2\relax}
  {%
    \let#2\relax%
    \cref@ifstreq{#1}{theorem}{%
      \cref@isinresetlist{#1}{test}%
      \if@cref@inresetlist%
        \def#2{test}%
      \fi%
    }{}%
  }{}{}

Thus, what probably need to be ensured are: 1) the corresponding counter for theorems (here theorem) is in the reset list of the parent counter (here \cl@test); 2) remove the current counter from other reset lists (not sure if you need this, but for my \SetTheorem to work properly this is probably necessary); 3) refresh \cref@resetby each time one touches the counter-related setting of the theorems.

This means that in our commands for counter aliasing (\@counteralias for you, \crthm_counter_alias:nn for me), we probably need to: 1) add #1 to the reset list of the parent counter of #2; 2) remove #1 from other reset lists (or at least remove from those parent counters that are modified by our commands); 3) inject new setting to \cref@resetby.

The main difficulty here is to get the parent counter of a given counter. What probably making this worse is that our commands for counter aliasing are messing up with the \cl@..., resulted in that one counter might now be inside multiple reset lists :(

I haven't got the time to look into it, but https://tex.stackexchange.com/a/44648 might be helpful.