bugGNU roff - Bugs: bug #65763, [long-term] strictly compare...

 
 

bug #65763: [long-term] strictly compare strings in macro packages

Submitter:  Dave <barx>
Submitted:  Mon 20 May 2024 04:27:44 AM UTC
   
 
Category:  Macro package - others/general Severity:  3 - Normal
Item Group:  Lint Status:  In Progress
Privacy:  Public Assigned to:  None
Open/Closed:  Open Planned Release:  None
* Mandatory Fields

Post a Comment

Add a New Comment Rich Markup
   

Discussion

Jump to the original submission

Wed 07 May 2025 07:18:49 PM UTC, comment #7: 


commit 4b0e0405dfec56879552ffae2488d9423566ad8d
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Wed May 7 10:52:44 2025 -0500

    [mdoc]: Address Savannah #65763 (`\?` comparison).

    * tmac/doc.tmac (Bd): Fix code style nit.  Perform "strict" string
      comparison when using formatted output comparison operator to peform
      matching on macro argument.  See subsection/node "Operators in
      Conditionals" in groff's Texinfo manual.

    See <https://savannah.gnu.org/bugs/?65763>.

    groff.info:
         Surround the comparands with '\?' to avoid formatting them; this
         causes them to be compared character by character, as with string
         comparisons in other programming languages.

              .ie "\?|\?"\?\fR|\fP\?" true
              .el false
                  => false

         Since GNU 'troff' reads comparands protected with '\?' in copy mode
         (*note Copy Mode::), they need not even be valid 'groff' syntax.
         The escape character is still lexically recognized, however, and
         consumes the next character.


G. Branden Robinson <gbranden>
Group administrator
Wed 07 May 2025 03:48:18 PM UTC, comment #6: 

Recategorizing as "Lint" since it's a code style issue, but raising severity to "Normal" as an especially salutary one.

G. Branden Robinson <gbranden>
Group administrator
Wed 07 May 2025 03:47:11 PM UTC, comment #5: 

Tagging as a "long-term" effort.

G. Branden Robinson <gbranden>
Group administrator
Tue 10 Dec 2024 03:54:49 AM UTC, comment #4: 

comment #2:

> as groff moves toward supporting such languages in other ways
> (the fixed bug #63076, the pending bug #62830).


Both now fixed!

> This breaks back compatibility with ~'s current use as a synonym
> (along with myriad other characters) for the ' conditional operator.


Such compatibility issues can come up in unexpected places: bug #66481 shows an example of a long-working delimiter surprising a longtime roff user when its meaning was changed.  And ~ seems more likely than | to be in use as a delimiter, since it doesn't carry other meanings in other contexts.

(The discussion of adding syntax to the parser for string comparison is not strictly on topic for this ticket, which concerns itself only with existing string comparisons in macro packages.  But there's no ticket (yet) for the former issue, and no one seems completely sold on the idea, so I hesitate to open one.  It probably needs to be vetted on the email list first.)

Dave <barx>
Group Member
Wed 19 Jun 2024 04:12:08 AM UTC, comment #3: 

("cc" bug that affects this ticket (and probably others) reported in http://savannah.n ... g/support/?111079)

Dave <barx>
Group Member
Wed 19 Jun 2024 03:43:40 AM UTC, comment #2: 

comment #1:

> I suspect this may be low priority,


I agree.

> perhaps so low that it's not worth doing, because as deployed,
> all of groff's full-service macro packages are prepared to
> format documents using the basic Latin character set.


Although documents in languages using Latin alphabets are likely groff's biggest use, I think it's hard to justify declining to fix (as opposed to deferring fixing) known bugs when it's used for non-Latin-alphabet languages, especially as groff moves toward supporting such languages in other ways (the fixed bug #63076, the pending bug #62830).

> I'm also still a bit undecided whether we should have a
> syntactically sweeter way of doing string comparisons.


Like you, I dislike the idea of adding basic language constructs to do something the language can already do, and also find the way the language already does this rather ugly.  So I'm no help in deciding.

> It grabs the symbol '~' for use as an operator in a conditional
> expression


This breaks back compatibility with ~'s current use as a synonym (along with myriad other characters) for the ' conditional operator.  That doesn't disqualify it, IMHO, but it is another strike against it.

> I suggest that nothing about it is a priority for 1.24.


I agree with this too.  Lowered the severity to reflect this.

Dave <barx>
Group Member
Wed 05 Jun 2024 03:21:42 AM UTC, comment #1: 

I suspect this may be low priority, perhaps so low that it's not worth doing, because as deployed, all of groff's full-service macro packages are prepared to format documents using the basic Latin character set.

I'm also still a bit undecided whether we should have a syntactically sweeter way of doing string comparisons.

Here's a proof of concept I did back when discussing this issue with Deri.

It's pretty simple.  It grabs the symbol '~' for use as an operator in a conditional expression and then uses the same infrastructure that's already in place to handle `\?`.


$ git stash show -p 1
diff --git a/src/roff/troff/input.cpp b/src/roff/troff/input.cpp
index 8d828a01e..7288f31c5 100644
--- a/src/roff/troff/input.cpp
+++ b/src/roff/troff/input.cpp
@@ -6002,6 +6002,25 @@ static bool do_if_request()
     }
     result = is_abstract_style(nm);
   }
+  else if (c == '~') {
+    tok.next();
+    symbol nm1 = get_name(true /* required */);
+    symbol nm2 = get_name(true /* required */);
+    if ((nm1.is_null() || nm2.is_null())) {
+      skip_branch();
+      return false;
+    }
+    macro *rm1 = static_cast<macro *>(request_dictionary.lookup(nm1));
+    macro *rm2 = static_cast<macro *>(request_dictionary.lookup(nm2));
+    macro *m1 = rm1->to_macro();
+    macro *m2 = rm2->to_macro();
+    if ((0 /* nullptr */ == m1) || (0 /* nullptr */ == m2)) {
+      error("cannot perform equality comparison on a request");
+      skip_branch();
+      return false;
+    }
+    result = (*m1 == *m2);
+  }
   else if (tok.is_space())
     result = false;
   else if (tok.is_usable_as_delimiter()) {


Not sure what the best way forward is.  I suggest that nothing about it is a priority for 1.24.

G. Branden Robinson <gbranden>
Group administrator
Mon 20 May 2024 04:27:44 AM UTC, original submission:  

Bug #64155 fixed fallbacks.tmac and troffrc-end to make tests that were intended only to compare strings do so without formatting the comparands, so that such comparisons work regardless of whether the current font contains all the characters in the strings.

It turns out most of the other macro packages need this refinement as well.

$ groff -ms -fZD < /dev/null
troff:tmac/s.tmac:2203: warning: character 'p' not defined
troff:tmac/s.tmac:2203: warning: character 's' not defined
troff:tmac/s.tmac:2203: warning: character 'h' not defined
troff:tmac/s.tmac:2203: warning: character 't' not defined
troff:tmac/s.tmac:2203: warning: character 'm' not defined
troff:tmac/s.tmac:2203: warning: character 'l' not defined
$ groff -mm -fZD < /dev/null
troff:contrib/mm/m.tmac:1294: warning: character 'p' not defined
troff:contrib/mm/m.tmac:1294: warning: character 's' not defined
troff:contrib/mm/m.tmac:1294: warning: character 'h' not defined
troff:contrib/mm/m.tmac:1294: warning: character 't' not defined
troff:contrib/mm/m.tmac:1294: warning: character 'm' not defined
troff:contrib/mm/m.tmac:1294: warning: character 'l' not defined
$ groff -me -fZD < /dev/null
troff:tmac/e.tmac:115: warning: character 'p' not defined
troff:tmac/e.tmac:115: warning: character 's' not defined
troff:tmac/e.tmac:115: warning: character 'h' not defined
troff:tmac/e.tmac:115: warning: character 't' not defined
troff:tmac/e.tmac:115: warning: character 'm' not defined
troff:tmac/e.tmac:115: warning: character 'l' not defined

(-fZD is a useful test, as this font ships with groff, but a likelier real-world scenario is users formatting documents containing only CJK characters.)

-man, -mdoc, and -mom exhibit similar behavior.  The first two of those operate in a specialized domain where the problem may not be applicable, and -mom has her own set of rules that may preclude specifying -f on the command line.  Peter is cc:ed so he can address this.

I didn't test all the auxiliary packages, but, for example, pdf.tmac and ps.tmac are clean while tty.tmac is not.

Dave <barx>
Group Member

 

Attached Files

This item currently has no attached files.

(Note: upload size limit is set to 4.0MiB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

 

Dependencies

This item does not depend on any other items.

No items depend on this one.

 

Mail Notification Carbon-Copy List

Carbon-Copy List
  • -email is unavailable- added by gbranden (Posted a comment)
  • -email is unavailable- added by barx (Submitted the item)
  • -email is unavailable- added by barx (-mom maintainer)
  •  

    Votes

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

    History

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2025-05-07 gbranden Severity2 - Minor 3 - Normal
        Item GroupWarning/Suspicious behaviour Lint
    2025-05-07 gbranden StatusNone In Progress
        Summarystrictly compare strings in macro packages [long-term] strictly compare strings in macro packages
    2024-06-19 barx Severity3 - Normal 2 - Minor
    2024-06-19 barx Severity2 - Minor 3 - Normal
    2024-06-19 barx Severity3 - Normal 2 - Minor
    2024-06-05 gbranden SummaryDo actual string comparisons in macro packages strictly compare strings in macro packages
    2024-05-20 barx Carbon-Copy- Added ptpi

    Back to the top

    Powered by Savane 3.16-a7ba.
    Corresponding source code