GNU bug report logs - #76851
29.4; cperl-mode builtin fcn indent bug and fix

Previous Next

Full log

View this message in rfc822 format

From: Harald Jörg <haj <at> posteo.de>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: John Ciolfi <ciolfi <at> mathworks.com>, 76851 <at> debbugs.gnu.org
Subject: bug#76851: 29.4; cperl-mode builtin fcn indent bug and fix
Date: Sat, 08 Mar 2025 18:21:50 +0000

Stefan Kangas <stefankangas <at> gmail.com> writes:

> Harald Jörg <haj <at> posteo.de> writes:
>
>> Stefan Kangas <stefankangas <at> gmail.com> writes:
>>
>>> Harald Jörg <haj <at> posteo.de> writes:
>>>
>>>> Stefan Kangas <stefankangas <at> gmail.com> writes:
>>>>
>>>>> John Ciolfi via "Bug reports for GNU Emacs, the Swiss army knife of text
>>>>> editors" <bug-gnu-emacs <at> gnu.org> writes:
>>>>>
>>>>>> Hi
>>>>>>
>>>>>> Given this perl file:
>>>>>>
>>>>>>     sub test {
>>>>>>         exec '/bin/echo',
>>>>>>             'Your arguments are: ', @ARGV;
>>>>>>     }
>>>>>>
>>>>>>     sub exec_fcn {
>>>>>>     }
>>>>>>
>>>>>>         sub other {
>>>>>>         }
>>>>>>
>>>>>> The 'sub other' is indented incorrectly (and all code following it).
>>>>>>
>>>>>> The fix is in cperl-after-block-and-statement-beg to not match exec_, i.e. we should
>>>>>> not treat exec_fcn as a builtin. Attached is the fix.
>>>>>
>>>>> Harald, any comments on this patch?
>>>>
>>>> Yes :)
>>>>
>>>> I confirm this is a bug, and one of those nasty ones which affect all
>>>> following code.
>>>>
>>>> I suggest a slightly improved patch.  Instead of replacing \> (end of
>>>> word) by [[:space:]] it should be replaced by \_> (end of symbol).
>>>> A space is not required after the keywords in the list.  In the case of
>>>> exec, this is handled elsewhere, but there are keywords in the list
>>>> where it matters.
>>>>
>>>> With [[:space:]], the following indentation happens:
>>>>
>>>>     my %h = map{$_=>1}
>>>>     @ARGV;
>>>>
>>>> With \_>, @ARGV is correctly indented as a continuation line:
>>>>
>>>>     my %h = map{$_=>1}
>>>>         @ARGV;
>>>>
>>>> This is less severe than the bug reported because it does not affect
>>>> following code, but still should not happen.
>>>
>>> I recommend adding tests for the above.  I think we already have good
>>> erts tests in cperl-indents.erts, that could easily be expanded.
>>
>> Sure, I plan to do that.  CPerl indentation with its multitude of
>> options is one of its messier corners, without extensive tests it is
>> dangerous to change anything.
>>
>> How do we proceed? Shall I commit my patch to emacs-30 together with
>> tests?
>
> I don't think I can fully assess the risks of installing or not
> installing this change, as cperl-mode is quite complex and I'm not
> too familiar with it.

Replacing end-of-word by end-of-symbol here is very low risk.  The same
change has been applied to other regular expressions in the past,
e.g. to fix Bug#18985.  The function patched here is one of the last
"tie-breakers" to distinguish whether a } character ends a statement.

> If this is a regression, we would tend to favor installing the fix on
> the release branch.  Otherwise, especially if this is a messy corner and
> we feel that it's risky to change, perhaps we should install it on
> master.

I _guess_ this bug sits here since 1999, commit 8f2222484972.  Before
that, the underscore character was considered a word character by CPerl
mode, so the regexp was "correct" unless someone fiddled with the option
cperl-under-as-char (deprecated since Emacs 24.4, and its replacement
superword-mode affects only movement, not regexps).

> What is your estimation of this issue?  Do you consider it important to
> fix on the release branch?

The chance to hit this bug is low, as indicated by the fact that it
didn't surface for that long.  I find it nasty, because it affects the
following code ... until it finds an extra semicolon which it takes as
end of the statement.

So, that's also a workaround: Add a semicolon after the closing brace.
    sub exec_fcn {...};

So, to summarize:
 + Risk: Very low.
 + Probability to hit: Very low.
 + Bug impact: annoying.
 + Workaround: exists and is easy (albeit ugly).

Pushing it to master works for me just fine, too.
-- 
Cheers,
haj

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.