bug#21409: 24.5; Wrong syntactic information for two line statement in an arglist

bug#21409: 24.5; Wrong syntactic information for two line statement in an arglist

[Gulshan Singh]

[-- Attachment #1: Type: text/plain, Size: 6379 bytes --]

In c-mode (and all derivatives), the following code has the wrong
syntactic information (at least, in my opinion):

foo(bar
    .baz()
    .qux());

Putting point at `.baz()` and pressing C-c C-s shows it as an
`arglist-cont-nonempty`, when I'd expect it to be a
`statement-cont`. This causes the code to have the wrong indentation, as
above I would like to have the continued statements to be indented one
c-basic-offset, not aligned to the opening brace.



In GNU Emacs 24.5.1 (x86_64-apple-darwin14.5.0, NS apple-appkit-1348.17)
 of 2015-08-24 on gulshan-mbp1
Windowing system distributor `Apple', version 10.3.1348
Configured using:
 `configure --prefix=/usr/local/Cellar/emacs/24.5
 --enable-locallisppath=/usr/local/share/emacs/site-lisp
 --infodir=/usr/local/Cellar/emacs/24.5/share/info/emacs --with-xml2
 --without-dbus --without-gnutls --with-ns --disable-ns-self-contained'

Important settings:
  locale-coding-system: utf-8-unix

Major mode: C/l

Minor modes in effect:
  yas-global-mode: t
  yas-minor-mode: t
  global-syntax-subword-mode: t
  syntax-subword-mode: t
  global-flycheck-mode: t
  flycheck-mode: t
  which-function-mode: t
  global-company-mode: t
  company-mode: t
  flx-ido-mode: t
  ido-ubiquitous-mode: t
  global-diff-hl-mode: t
  diff-auto-refine-mode: t
  winner-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  whitespace-mode: t
  global-anzu-mode: t
  anzu-mode: t
  projectile-global-mode: t
  projectile-mode: t
  shell-dirtrack-mode: t
  volatile-highlights-mode: t
  global-hl-line-mode: t
  recentf-mode: t
  savehist-mode: t
  show-smartparens-global-mode: t
  show-smartparens-mode: t
  smartparens-mode: t
  global-auto-revert-mode: t
  delete-selection-mode: t
  prelude-global-mode: t
  prelude-mode: t
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t
  abbrev-mode: t

Recent messages:
Quit
Mark set [2 times]
Syntactic analysis: ((arglist-cont-nonempty 72 75))
Quit
use of undeclared identifier 'bar'
Mark set
Quit
byte-code: Command attempted to use minibuffer while in minibuffer
Quit


Load-path shadows:
None found.

Features:
(shadow sort eieio-opt emacsbug message rfc822 mml mml-sec mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
sendmail rfc2047 rfc2045 ietf-drums mail-utils mail-extr cus-edit
cus-start cus-load easy-kill misearch multi-isearch js2-imenu-extras
tramp-cmds ffap url-parse url-vars cc-langs xhp-mode derived xhp-indent
php-mode speedbar sb-image ezimage dframe flymake add-log tramp-cache
tramp-sh company-anaconda anaconda-mode f s ucs-normalize json-rpc
python-el-fgallina-expansions smartparens-python python rainbow-mode
color rainbow-delimiters elisp-slime-nav yasnippet appt diary-lib
diary-loaddefs cal-menu calendar cal-loaddefs syntax-subword superword
subword prelude-xml nxml-mode-expansions html-mode-expansions sgml-mode
rng-nxml rng-valid rng-loc rng-uri rng-parse nxml-parse rng-match rng-dt
rng-util rng-pttrn nxml-ns nxml-mode nxml-outln nxml-rap nxml-util
nxml-glyph nxml-enc xmltok prelude-web web-mode-expansions
smartparens-html web-mode disp-table prelude-shell sh-script smie
executable prelude-python prelude-js js2-mode-expansions js2-mode
js2-old-indent js-mode-expansions js json cc-mode-expansions cc-mode
cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars
cc-defs prelude-emacs-lisp prelude-lisp prelude-c prelude-programming
flycheck find-func help-mode rx subr-x which-func prelude-company
company-files company-oddmuse company-keywords company-etags
company-gtags company-dabbrev-code company-dabbrev company-capf
company-cmake company-xcode company-clang company-semantic company-eclim
company-template company-css company-nxml company-bbdb company
prelude-ido smex flx-ido flx ido-ubiquitous ido-completing-read+
prelude-osx exec-path-from-shell prelude-global-keybindings
prelude-editor operate-on-number calc-bin calc-ext calc calc-loaddefs
calc-macs diff-hl smartrep vc-dir ewoc vc vc-dispatcher diff-mode
easy-mmode winner undo-tree diff esh-var esh-io esh-cmd esh-opt esh-ext
esh-proc esh-arg eldoc esh-groups eshell esh-module esh-mode esh-util
re-builder whitespace tabify browse-kill-ring midnight ediff-merg
ediff-wind ediff-diff ediff-mult ediff-help ediff-init ediff-util ediff
dired-x dired anzu avy projectile compile ibuf-ext ibuffer bookmark pp
expand-region text-mode-expansions er-basic-expansions
expand-region-core expand-region-custom flyspell ispell tramp
tramp-compat auth-source gnus-util mm-util mail-prsvr password-cache
tramp-loaddefs trampver shell pcomplete comint ansi-color format-spec
etags ring volatile-highlights hl-line windmove recentf tree-widget
wid-edit savehist saveplace diminish edmacro kmacro smartparens-config
smartparens autorevert filenotify delsel prelude-mode prelude-core imenu
epl ido pcase ov dash thingatpt prelude-ui smart-mode-line mule-util
rich-minority zenburn-theme prelude-custom prelude-packages finder-inf
eieio byte-opt bytecomp byte-compile cl-extra cconv eieio-core advice
help-fns cl-macs info easymenu package epg-config cl gv cl-loaddefs
cl-lib time-date tooltip electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel ns-win tool-bar dnd fontset image regexp-opt
fringe tabulated-list newcomment lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process cocoa ns
multi-tty emacs)

Memory information:
((conses 16 892477 436428)
 (symbols 48 54665 2)
 (miscs 40 1391 5733)
 (strings 32 135135 214576)
 (string-bytes 1 3737179)
 (vectors 16 155851)
 (vector-slots 8 4232856 614143)
 (floats 8 28966 6540)
 (intervals 56 3573 9245)
 (buffers 960 35))

[-- Attachment #2: Type: text/html, Size: 8188 bytes --]

bug#21409: 24.5; Wrong syntactic information for two line statement in an arglist

[Lars Ingebrigtsen]

Gulshan Singh <gsingh2011@gmail.com> writes:

> In c-mode (and all derivatives), the following code has the wrong
> syntactic information (at least, in my opinion):
>
> foo(bar
>     .baz()
>     .qux());
>
> Putting point at `.baz()` and pressing C-c C-s shows it as an
> `arglist-cont-nonempty`, when I'd expect it to be a
> `statement-cont`. This causes the code to have the wrong indentation, as
> above I would like to have the continued statements to be indented one
> c-basic-offset, not aligned to the opening brace.

(This bug report unfortunately got no response at the time.)

I'm not sure how that should be indented, really -- the current
indentation looks reasonable to me, I think?

Perhaps Alan (added to the Cc's) has an opinion here.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#21409: 24.5; Wrong syntactic information for two line statement in an arglist

[Gulshan Singh]

[-- Attachment #1: Type: text/plain, Size: 1482 bytes --]

I know this is an old bug report, but I just realized it got a response,
and it seems like the behavior hasn't changed.

On Thu, Dec 3, 2020 at 3:07 AM Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Gulshan Singh <gsingh2011@gmail.com> writes:
>
> > In c-mode (and all derivatives), the following code has the wrong
> > syntactic information (at least, in my opinion):
> >
> > foo(bar
> >     .baz()
> >     .qux());
> >
> > Putting point at `.baz()` and pressing C-c C-s shows it as an
> > `arglist-cont-nonempty`, when I'd expect it to be a
> > `statement-cont`. This causes the code to have the wrong indentation, as
> > above I would like to have the continued statements to be indented one
> > c-basic-offset, not aligned to the opening brace.
>
> (This bug report unfortunately got no response at the time.)
>
> I'm not sure how that should be indented, really -- the current
> indentation looks reasonable to me, I think?
>

It's definitely reasonable, but it's not what I'd prefer, which would be
this:

foo(bar
      .baz()
      .qux());

`.baz()` and `.qux()` are indented two spaces (my value for
`c-basic-offset`) from the start of `bar`, as opposed to aligned with
`bar`. This matches what happens if the call to `foo` isn't there:

bar
  .baz()
  .qux();

In any case, regardless of what indentation one would prefer for this case,
the issue remains that `c-show-syntactic-information` should be showing
`statement-cont` instead of `arglist-cont-nonempty` at `.baz()`.

[-- Attachment #2: Type: text/html, Size: 2000 bytes --]

bug#21409: 24.5; Wrong syntactic information for two line statement in an arglist

[Alan Mackenzie]

Hello, Gulshan.

Sorry I missed your bug report back in 2015.

On Fri, Mar 11, 2022 at 17:52:38 -0800, Gulshan Singh wrote:
> I know this is an old bug report, but I just realized it got a response,
> and it seems like the behavior hasn't changed.

Also, a big thank you for cutting the C fragment down to an easy to work
with minimum.

> On Thu, Dec 3, 2020 at 3:07 AM Lars Ingebrigtsen <larsi@gnus.org> wrote:

> > Gulshan Singh <gsingh2011@gmail.com> writes:

> > > In c-mode (and all derivatives), the following code has the wrong
> > > syntactic information (at least, in my opinion):

> > > foo(bar
> > >     .baz()
> > >     .qux());

> > > Putting point at `.baz()` and pressing C-c C-s shows it as an
> > > `arglist-cont-nonempty`, when I'd expect it to be a
> > > `statement-cont`.

I'm afraid I can't agree with you here.  The bar.baz().qux() is an
expression, not a statement, as it is lacking the terminating ; which
would make it a statement.  I think the arglist-cont-nonempty is correct.
It is more specific than statement-cont, which could only have the start
of foo as its anchor position.

> > > This causes the code to have the wrong indentation, as above I
> > > would like to have the continued statements to be indented one
> > > c-basic-offset, not aligned to the opening brace.

I think the best solution to the problem would be to write a new Line-Up
function for this particular scenario, and to make it available to users
to insert into the c-offsets-alist entry for arglist-cont-nonempty.  The
page "Customizing Indentation" in the CC Mode manual is pertinent here.

But first, we need to firm up the specification.  What, precisely, will
trigger this new Line-Up function?

What about the struct members not being functions:

    foo(bar
          .baz
	  .qux);

?  What about the . operator being at the end of the previous line:

    foo(bar.
          baz().
	  qux());

?  What is the primary construct which should trigger the new Line-Up
function?  Am I right in thinking it's the . operator combined with line
breaks, and nothing else?  What about, for example, the ? : operators:

    foo(bar
          ? baz()
	  : qux());

?  This is getting kind of complicated.  ;-)  Sorry.

> > (This bug report unfortunately got no response at the time.)

> > I'm not sure how that should be indented, really -- the current
> > indentation looks reasonable to me, I think?


> It's definitely reasonable, but it's not what I'd prefer, which would be
> this:

> foo(bar
>       .baz()
>       .qux());

> `.baz()` and `.qux()` are indented two spaces (my value for
> `c-basic-offset`) from the start of `bar`, as opposed to aligned with
> `bar`. This matches what happens if the call to `foo` isn't there:

> bar
>   .baz()
>   .qux();

> In any case, regardless of what indentation one would prefer for this case,
> the issue remains that `c-show-syntactic-information` should be showing
> `statement-cont` instead of `arglist-cont-nonempty` at `.baz()`.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#21409: 24.5; Wrong syntactic information for two line statement in an arglist

[Alan Mackenzie]

Hello again, Gulshan.

On Sat, Mar 12, 2022 at 11:32:50 +0000, Alan Mackenzie wrote:

> Sorry I missed your bug report back in 2015.

> On Fri, Mar 11, 2022 at 17:52:38 -0800, Gulshan Singh wrote:
> > On Thu, Dec 3, 2020 at 3:07 AM Lars Ingebrigtsen <larsi@gnus.org> wrote:

> > > Gulshan Singh <gsingh2011@gmail.com> writes:

> > > > In c-mode (and all derivatives), the following code has the wrong
> > > > syntactic information (at least, in my opinion):

> > > > foo(bar
> > > >     .baz()
> > > >     .qux());

[ .... ]

> I think the best solution to the problem would be to write a new Line-Up
> function for this particular scenario, and to make it available to users
> to insert into the c-offsets-alist entry for arglist-cont-nonempty.  The
> page "Customizing Indentation" in the CC Mode manual is pertinent here.

> But first, we need to firm up the specification.  What, precisely, will
> trigger this new Line-Up function?

I've come up with an answer to that question.  On _any_ argument
continued onto the next line, we indent it c-basic-offset from the
_first_ argument.  This is easy to implement, since it's a minor
variation on c-lineup-arglist.  See the following patch for an example
of what that does.

[ .... ]

> > It's definitely reasonable, but it's not what I'd prefer, which would be
> > this:

> > foo(bar
> >       .baz()
> >       .qux());

> > `.baz()` and `.qux()` are indented two spaces (my value for
> > `c-basic-offset`) from the start of `bar`, as opposed to aligned with
> > `bar`. This matches what happens if the call to `foo` isn't there:

> > bar
> >   .baz()
> >   .qux();

I've hacked up the following patch, which introduces the new Line-Up
function c-lineup-arglist-+.  To use it (temporarily) do C-c C-o RET on
the .baz() line, and change the setting for arglist-cont-nonempty from

    (c-lineup-gcc-asm-reg c-lineup-arglist)

to

    (c-lineup-gcc-asm-reg c-lineup-arglist-+ c-lineup-arglist)

..  Note that c-lineup-arglist-+ is a function which returns nil to mean
"not appropriate here", so it must be in a list, not in the last
position.  This is all better explained in the CC Mode manual on page
"c-offsets-alist".

If this patch does what you want, you can then incorporate the new
Line-Up function into your CC Mode style, or however else you set up
your indentation.  If you want any help with this, feel free to ask on
this list, or on bug-cc-mode@gnu.org.

Here's the patch.  It should apply to either the latest version of
stand-alone CC Mode, or the version in the Emacs master branch.  Please
apply the patch, byte compile the changed file (or all of CC Mode), and
make the amendment to your indentation setup noted above.  (If you want
any help with any of this, feel free to send me private email).  Then
please let us know if this patch does the Right Thing.  Thanks!



diff -r 1a0681da2be1 cc-align.el
--- a/cc-align.el	Thu Feb 10 16:46:58 2022 +0000
+++ b/cc-align.el	Sun Mar 13 13:35:56 2022 +0000
@@ -207,6 +207,58 @@
 	  (vector (current-column)))))))
 
 ;; Contributed by Kevin Ryde <user42@zip.com.au>.
+(defun c-lineup-argcont-1 (elem)
+  ;; Move to the start of the current arg and return non-nil, otherwise
+  ;; return nil.
+  (beginning-of-line)
+
+  (when (eq (car elem) 'arglist-cont-nonempty)
+    ;; Our argument list might not be the innermost one.  If it
+    ;; isn't, go back to the last position in it.  We do this by
+    ;; stepping back over open parens until we get to the open paren
+    ;; of our argument list.
+    (let ((open-paren (c-langelem-2nd-pos c-syntactic-element))
+	  (paren-state (c-parse-state)))
+      (while (not (eq (car paren-state) open-paren))
+	(unless (consp (car paren-state)) ;; ignore matched braces
+	  (goto-char (car paren-state)))
+	(setq paren-state (cdr paren-state)))))
+
+  (let ((start (point)) c)
+
+    (when (bolp)
+      ;; Previous line ending in a comma means we're the start of an
+      ;; argument.  This should quickly catch most cases not for us.
+      ;; This case is only applicable if we're the innermost arglist.
+      (c-backward-syntactic-ws)
+      (setq c (char-before)))
+
+    (unless (eq c ?,)
+      ;; In a gcc asm, ":" on the previous line means the start of an
+      ;; argument.  And lines starting with ":" are not for us, don't
+      ;; want them to indent to the preceding operand.
+      (let ((gcc-asm (save-excursion
+		       (goto-char start)
+		       (c-in-gcc-asm-p))))
+	(unless (and gcc-asm
+		     (or (eq c ?:)
+			 (save-excursion
+			   (goto-char start)
+			   (looking-at "[ \t]*:"))))
+
+	  (c-lineup-argcont-scan (if gcc-asm ?:))
+	  t)))))
+
+(defun c-lineup-argcont-scan (&optional other-match)
+  ;; Find the start of an argument, for `c-lineup-argcont'.
+  (when (zerop (c-backward-token-2 1 t))
+    (let ((c (char-after)))
+      (if (or (eq c ?,) (eq c other-match))
+	  (progn
+	    (forward-char)
+	    (c-forward-syntactic-ws))
+	(c-lineup-argcont-scan other-match)))))
+
 (defun c-lineup-argcont (elem)
   "Line up a continued argument.
 
@@ -221,56 +273,28 @@
 for the operands.
 
 Works with: arglist-cont, arglist-cont-nonempty."
-
   (save-excursion
-    (beginning-of-line)
+    (when (c-lineup-argcont-1 elem)
+      (vector (current-column)))))
 
-    (when (eq (car elem) 'arglist-cont-nonempty)
-      ;; Our argument list might not be the innermost one.  If it
-      ;; isn't, go back to the last position in it.  We do this by
-      ;; stepping back over open parens until we get to the open paren
-      ;; of our argument list.
-      (let ((open-paren (c-langelem-2nd-pos c-syntactic-element))
-	    (paren-state (c-parse-state)))
-	(while (not (eq (car paren-state) open-paren))
-	  (unless (consp (car paren-state)) ;; ignore matched braces
-	    (goto-char (car paren-state)))
-	  (setq paren-state (cdr paren-state)))))
+(defun c-lineup-argcont-+ (langelem)
+  "Indent an argument continuation `c-basic-offset' in from the first argument.
 
-    (let ((start (point)) c)
-
-      (when (bolp)
-	;; Previous line ending in a comma means we're the start of an
-	;; argument.  This should quickly catch most cases not for us.
-	;; This case is only applicable if we're the innermost arglist.
-	(c-backward-syntactic-ws)
-	(setq c (char-before)))
+foo (xyz, uvw, aaa + bbb + ccc
+         + ddd + eee + fff);    <- c-lineup-argcont-+
+     <-->                          c-basic-offset
 
-      (unless (eq c ?,)
-	;; In a gcc asm, ":" on the previous line means the start of an
-	;; argument.  And lines starting with ":" are not for us, don't
-	;; want them to indent to the preceding operand.
-	(let ((gcc-asm (save-excursion
-			 (goto-char start)
-			 (c-in-gcc-asm-p))))
-	  (unless (and gcc-asm
-		       (or (eq c ?:)
-			   (save-excursion
-			     (goto-char start)
-			     (looking-at "[ \t]*:"))))
+Only continuation lines like this are touhced, nil being returned
+on lines which are the start of an argument.
 
-	    (c-lineup-argcont-scan (if gcc-asm ?:))
-	    (vector (current-column))))))))
-
-(defun c-lineup-argcont-scan (&optional other-match)
-  ;; Find the start of an argument, for `c-lineup-argcont'.
-  (when (zerop (c-backward-token-2 1 t))
-    (let ((c (char-after)))
-      (if (or (eq c ?,) (eq c other-match))
-	  (progn
-	    (forward-char)
-	    (c-forward-syntactic-ws))
-	(c-lineup-argcont-scan other-match)))))
+Works with: arglist-cont, arglist-cont-nonempty."
+  (save-excursion
+    (when (c-lineup-argcont-1 langelem)	; Check we've got a continued argument...
+      ;; ... but ignore the position found.
+      (goto-char (c-langelem-2nd-pos c-syntactic-element))
+      (forward-char)
+      (c-forward-syntactic-ws)
+      (vector (+ (current-column) c-basic-offset)))))
 
 (defun c-lineup-arglist-intro-after-paren (_langelem)
   "Line up a line to just after the open paren of the surrounding paren


-- 
Alan Mackenzie (Nuremberg, Germany).

bug#21409: 24.5; Wrong syntactic information for two line statement in an arglist

[Gulshan Singh]

Hi Alan, thanks for the patch! I've been very busy, but I just got
around to applying it and testing it out.

> I've hacked up the following patch, which introduces the new Line-Up
> function c-lineup-arglist-+.  To use it (temporarily) do C-c C-o RET on
> the .baz() line, and change the setting for arglist-cont-nonempty from
>
>     (c-lineup-gcc-asm-reg c-lineup-arglist)
>
> to
>
>     (c-lineup-gcc-asm-reg c-lineup-arglist-+ c-lineup-arglist)

I think you meant `(c-lineup-gcc-asm-reg c-lineup-argcont-+
c-lineup-arglist)` (or you meant to define the function name as
`c-lineup-arglist-+` instead of `c-lineup-argcont-+`, not sure).

In any case, I tested it and it works great! Is this patch something
that could be merged upstream?

bug#21409: 24.5; Wrong syntactic information for two line statement in an arglist

[Alan Mackenzie]

Hello, Gulshan.

Sorry I've been a bit busy the last two weeks, even if mainly on other
Emacs things.

On Sat, Apr 09, 2022 at 14:43:32 -0700, Gulshan Singh wrote:
> Hi Alan, thanks for the patch! I've been very busy, but I just got
> around to applying it and testing it out.

> > I've hacked up the following patch, which introduces the new Line-Up
> > function c-lineup-arglist-+.  To use it (temporarily) do C-c C-o RET on
> > the .baz() line, and change the setting for arglist-cont-nonempty from

> >     (c-lineup-gcc-asm-reg c-lineup-arglist)

> > to

> >     (c-lineup-gcc-asm-reg c-lineup-arglist-+ c-lineup-arglist)

> I think you meant `(c-lineup-gcc-asm-reg c-lineup-argcont-+
> c-lineup-arglist)` (or you meant to define the function name as
> `c-lineup-arglist-+` instead of `c-lineup-argcont-+`, not sure).

I think you're right, here!

> In any case, I tested it and it works great! Is this patch something
> that could be merged upstream?

Thanks!

I'm part way through more thorough testing, and I'm hoping to commit it
this weekend (after having updated the documentation).  It will most
definitely appear in the next major Emacs version, Emacs 29.1, when it
appears (in around 1 - 2 years time).

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#21409: 24.5; Wrong syntactic information for two line statement in an arglist

[Alan Mackenzie]

Hello again Gulshan.

On Sat, Apr 09, 2022 at 14:43:32 -0700, Gulshan Singh wrote:
> Hi Alan, thanks for the patch! I've been very busy, but I just got
> around to applying it and testing it out.

> > I've hacked up the following patch, ....

[ .... ]

> In any case, I tested it and it works great! Is this patch something
> that could be merged upstream?

Thanks once again for the testing.

I've committed the patch (together with an update to the CC Mode manual)
to both standalone CC Mode and the Emacs repository master branch.

I'm closing the bug with this post.

-- 
Alan Mackenzie (Nuremberg, Germany).