bug#46301: 28.0.50; cc-mode: add support for c++ lambda expression

bug#46301: 28.0.50; cc-mode: add support for c++ lambda expression

[Utkarsh Singh]

[-- Attachment #1.1: Type: text/plain, Size: 1277 bytes --]

Hi,

This bugs only occurs when we assign lambda expression as default
arguments in C++.  I know there are better ways of using lambda but I
was just playing with both C++ and Emacs on a hobby project.

Consider this C++ code:

template <typename T>
void bubble_sort(std :: vector<T> &v, bool cmp(T, T)=[](T a, T b){return a < b;})
{
    bool swapped = true;
    for(size_t i = 0; i < v.size() && swapped; i++) {
	swapped = false;
	for(size_t j = 0; j < v.size()-i; j++){
	    if(cmp(v[j+1], v[j])) {
		std::swap(v[j], v[j+1]);
		swapped=true;
	    }
	}
    }
}

Here cmp takes a lambda as default argument and if in c++-mode if we try
using C-M-e(c-end-of-defun) it will get trapped into lambda.

To solve this is added c-forward-to-nth

((memq where '(at-function-end outwith-function at-header in-header))
    (when (c-syntactic-re-search-forward "{" nil 'eob)
      (backward-char)
      (forward-sexp)

      ;; continue to move sexp if we are looking ) and ,
      (while (looking-at ")\\|,")
	(c-syntactic-re-search-forward "{" nil 'eob)
	(backward-char)
	(forward-sexp))

      (setq n (1- n))))

By this I am trying to move forward lambda's which are separated by ','
or ended with ')'.  I will also attaching the necessary diff to support
the issue.
--
Utkarsh Singh

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: support c++ lambda --]
[-- Type: text/x-patch, Size: 150 bytes --]

1970a1971,1976
> 
>       (while (looking-at ")\\|,")
> 	(c-syntactic-re-search-forward "{" nil 'eob)
> 	(backward-char)
> 	(forward-sexp))
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 519 bytes --]

bug#46301: 28.0.50; cc-mode: add support for c++ lambda expression

[Alan Mackenzie]

Hello, Utkarsh.

On Thu, Feb 04, 2021 at 21:17:22 +0530, Utkarsh Singh wrote:
> Hi,

> This bugs only occurs when we assign lambda expression as default
> arguments in C++.  I know there are better ways of using lambda but I
> was just playing with both C++ and Emacs on a hobby project.

> Consider this C++ code:

> template <typename T>
> void bubble_sort(std :: vector<T> &v, bool cmp(T, T)=[](T a, T b){return a < b;})
> {
>     bool swapped = true;
>     for(size_t i = 0; i < v.size() && swapped; i++) {
> 	swapped = false;
> 	for(size_t j = 0; j < v.size()-i; j++){
> 	    if(cmp(v[j+1], v[j])) {
> 		std::swap(v[j], v[j+1]);
> 		swapped=true;
> 	    }
> 	}
>     }
> }

> Here cmp takes a lambda as default argument and if in c++-mode if we try
> using C-M-e(c-end-of-defun) it will get trapped into lambda.

But from the end of that lambda, a further C-M-e takes point to the end
of bubble_sort.

Why do you think this is a bug?  The lambda form is a function, it is at
the top level (i.e. not within braces), so why shouldn't C-M-e stop at
its end?  That's a genuine question, by the way, not a rhetorical one.

If anything, the rather unsystematic behaviour of C-M-a near this lambda
function is of more concern.  And there's the worry that this example
will break other places where it's assumed that braces cannot occur in
parameter lists.

But of all the languages there are, trust C++ to be awkward.  ;-)

> To solve this is added c-forward-to-nth

[ patch received with thanks, but snipped. ]

> --
> Utkarsh Singh

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#46301: 28.0.50; cc-mode: add support for c++ lambda expression

[Utkarsh Singh]

Alan Mackenzie <acm@muc.de> writes:

> But from the end of that lambda, a further C-M-e takes point to the end
> of bubble_sort.
>
> Why do you think this is a bug?  The lambda form is a function, it is at
> the top level (i.e. not within braces), so why shouldn't C-M-e stop at
> its end?  That's a genuine question, by the way, not a rhetorical one.

I think there was some misunderstanding on my side as I was interpreting
c-end-of-defun as c-end-of-*named*-defun.  This means I was not
considering lambda as an unnamed function but just as a default
argument.

> If anything, the rather unsystematic behaviour of C-M-a near this lambda
> function is of more concern.  And there's the worry that this example
> will break other places where it's assumed that braces cannot occur in
> parameter lists.
>
> But of all the languages there are, trust C++ to be awkward.  ;-)

Yes, C++ can get really ugly sometimes.

The main moto behind my "feature request" was not solving the problem
logically but to be so called "contributor" of GNU Emacs.  Apology for
that.
-- 
Utkarsh Singh

bug#46301: 28.0.50; cc-mode: add support for c++ lambda expression

[Alan Mackenzie]

Hello, Utkarsh.

On Wed, Feb 24, 2021 at 09:37:08 +0530, Utkarsh Singh wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > But from the end of that lambda, a further C-M-e takes point to the end
> > of bubble_sort.

> > Why do you think this is a bug?  The lambda form is a function, it is at
> > the top level (i.e. not within braces), so why shouldn't C-M-e stop at
> > its end?  That's a genuine question, by the way, not a rhetorical one.

> I think there was some misunderstanding on my side as I was interpreting
> c-end-of-defun as c-end-of-*named*-defun.  This means I was not
> considering lambda as an unnamed function but just as a default
> argument.

I've looked into a bit more, and come to the conclusion that this
behaviour really is a bug.  At least C-M-a/e don't recognize other C++
lambda functions as functions, so it shouldn't with this one in the
parameter list, either.

Besides, lambda functions in C++ are typically short, so the need for
C-M-a/e to work on them seems less than the annoyance just moving 1 or 2
lines would cause.

> > If anything, the rather unsystematic behaviour of C-M-a near this lambda
> > function is of more concern.  And there's the worry that this example
> > will break other places where it's assumed that braces cannot occur in
> > parameter lists.

> > But of all the languages there are, trust C++ to be awkward.  ;-)

> Yes, C++ can get really ugly sometimes.

Indeed.

> The main moto behind my "feature request" was not solving the problem
> logically but to be so called "contributor" of GNU Emacs.  Apology for
> that.

No need to apologize!  On the contrary, you've reported a bug, which has
led to an improvement in the source code.  Without bug reports, CC Mode
would not be anywhere near what it is today.

I've spent the last couple of days fixing the handling of C-M-a/e in
cc-cmds.el.  It took rather more than the four line patch you proposed.
;-)  Could I ask you, please, to apply the following patch (which should
apply cleanly to the Emacs master branch), byte compile cc-cmds.el, load
it into Emacs, and test it for me on your real source code.  Then please
report back whether the bug is in fact fixed, and if not, what is still
wrong.  In the event you would like help with the patching or byte
compiling, feel free to send me private email.

Thanks!



diff -r 00245e7ea2c5 cc-cmds.el
--- a/cc-cmds.el	Tue Feb 23 10:58:22 2021 +0000
+++ b/cc-cmds.el	Thu Feb 25 20:15:09 2021 +0000
@@ -1621,7 +1621,7 @@
   ;;
   ;; This function might do hidden buffer changes.
   (save-excursion
-    (let* (kluge-start
+    (let* (kluge-start knr-start knr-res
 	   decl-result brace-decl-p
 	   (start (point))
 	   (paren-state (c-parse-state))
@@ -1652,63 +1652,39 @@
 		    (not (looking-at c-defun-type-name-decl-key))))))
 	'at-function-end)
        (t
-	;; Find the start of the current declaration.  NOTE: If we're in the
-	;; variables after a "struct/eval" type block, we don't get to the
-	;; real declaration here - we detect and correct for this later.
-
-	;;If we're in the parameters' parens, move back out of them.
-	(if least-enclosing (goto-char least-enclosing))
-	;; Kluge so that c-beginning-of-decl-1 won't go back if we're already
-	;; at a declaration.
-	(if (or (and (eolp) (not (eobp))) ; EOL is matched by "\\s>"
-		(not (looking-at
-"\\([;#]\\|\\'\\|\\s(\\|\\s)\\|\\s\"\\|\\s\\\\|\\s$\\|\\s<\\|\\s>\\|\\s!\\)")))
-	    (forward-char))
-	(setq kluge-start (point))
-	;; First approximation as to whether the current "header" we're in is
-	;; one followed by braces.
-	(setq brace-decl-p
-	      (save-excursion
-		(and (c-syntactic-re-search-forward "[;{]" nil t t)
-		     (or (eq (char-before) ?\{)
-			 (and c-recognize-knr-p
-			      ;; Might have stopped on the
-			      ;; ';' in a K&R argdecl.  In
-			      ;; that case the declaration
-			      ;; should contain a block.
-			      (c-in-knr-argdecl))))))
-	(setq decl-result
-	      (car (c-beginning-of-decl-1
-		    ;; NOTE: If we're in a K&R region, this might be the start
-		    ;; of a parameter declaration, not the actual function.
-		    ;; It might also leave us at a label or "label" like
-		    ;; "private:".
-		    (and least-enclosing ; LIMIT for c-b-of-decl-1
-			 (c-safe-position least-enclosing paren-state)))))
-
-	;; Has the declaration we've gone back to got braces?
-	(if (or (eq decl-result 'label)
-		(looking-at c-protection-key))
-	    (setq brace-decl-p nil))
-
-	(cond
-	 ((or (eq decl-result 'label)	; e.g. "private:" or invalid syntax.
-	      (= (point) kluge-start))	; might be BOB or unbalanced parens.
-	  'outwith-function)
-	 ((eq decl-result 'same)
-	  (if brace-decl-p
-	      (if (eq (point) start)
-		  'at-header
+	(if (and least-enclosing
+		 (eq (char-after least-enclosing) ?\())
+	    (c-go-list-forward least-enclosing))
+	(c-forward-syntactic-ws)
+	(setq knr-start (point))
+	(if (c-syntactic-re-search-forward "{" nil t t)
+	    (progn
+	      (backward-char)
+	      (cond
+	       ((or (progn
+		      (c-backward-syntactic-ws)
+		      (<= (point) start))
+		    (and c-recognize-knr-p
+			 (and (setq knr-res (c-in-knr-argdecl))
+			      (<= knr-res knr-start))))
 		'in-header)
-	    'outwith-function))
-	 ((eq decl-result 'previous)
-	  (if (and (not brace-decl-p)
-		   (c-in-function-trailer-p))
-	      'at-function-end
-	    'outwith-function))
-	 (t (error
-	     "c-where-wrt-brace-construct: c-beginning-of-decl-1 returned %s"
-	     decl-result))))))))
+	       ((and knr-res
+		     (goto-char knr-res)
+		     (c-backward-syntactic-ws))) ; Always returns nil.
+	       ((and (eq (char-before) ?\))
+		     (c-go-list-backward))
+		(c-syntactic-skip-backward "^;" start t)
+		(if (eq (point) start)
+		    (if (progn (c-backward-syntactic-ws)
+			       (memq (char-before) '(?\; ?} nil)))
+			(if (progn (c-forward-syntactic-ws)
+				   (eq (point) start))
+			    'at-header
+			  'outwith-function)
+		      'in-header)
+		  'outwith-function))
+	       (t 'outwith-function)))
+	  'outwith-function))))))
 
 (defun c-backward-to-nth-BOF-{ (n where)
   ;; Skip to the opening brace of the Nth function before point.  If
@@ -1730,9 +1707,11 @@
     (goto-char (c-least-enclosing-brace (c-parse-state)))
     (setq n (1- n)))
    ((eq where 'in-header)
-    (c-syntactic-re-search-forward "{")
-    (backward-char)
-    (setq n (1- n)))
+    (let ((encl-paren (c-least-enclosing-brace (c-parse-state))))
+      (if encl-paren (goto-char encl-paren))
+      (c-syntactic-re-search-forward "{" nil t t)
+      (backward-char)
+      (setq n (1- n))))
    ((memq where '(at-header outwith-function at-function-end in-trailer))
     (c-syntactic-skip-backward "^}")
     (when (eq (char-before) ?\})
@@ -1941,21 +1920,24 @@
     ;; The actual movement is done below.
     (setq n (1- n)))
    ((memq where '(at-function-end outwith-function at-header in-header))
-    (when (c-syntactic-re-search-forward "{" nil 'eob)
+    (if (eq where 'in-header)
+	(let ((pos (c-least-enclosing-brace (c-parse-state))))
+	  (if pos (c-go-list-forward pos))))
+    (when (c-syntactic-re-search-forward "{" nil 'eob t)
       (backward-char)
       (forward-sexp)
       (setq n (1- n))))
    (t (error "c-forward-to-nth-EOF-\\;-or-}: `where' is %s" where)))
 
-  (when (c-in-function-trailer-p)
-    (c-syntactic-re-search-forward ";" nil 'eob t))
-
   ;; Each time round the loop, go forward to a "}" at the outermost level.
   (while (and (> n 0) (not (eobp)))
     (when (c-syntactic-re-search-forward "{" nil 'eob)
       (backward-char)
       (forward-sexp)
       (setq n (1- n))))
+
+  (when (c-in-function-trailer-p)
+    (c-syntactic-re-search-forward ";" nil 'eob t))
   n)
 
 (defun c-end-of-defun (&optional arg)


> -- 
> Utkarsh Singh

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#46301: 28.0.50; cc-mode: add support for c++ lambda expression

[Utkarsh Singh]

Alan Mackenzie <acm@muc.de> writes:

> I've spent the last couple of days fixing the handling of C-M-a/e in
> cc-cmds.el.  It took rather more than the four line patch you proposed.
> ;-)  Could I ask you, please, to apply the following patch (which should
> apply cleanly to the Emacs master branch), byte compile cc-cmds.el, load
> it into Emacs, and test it for me on your real source code.  Then please
> report back whether the bug is in fact fixed, and if not, what is still
> wrong.  In the event you would like help with the patching or byte
> compiling, feel free to send me private email.
>

Thanks now issue is fixed and code itself looks pretty solid.

Due to my in-experience in Elisp and comprehensive cc-mode library I was
unable to deep dive into the patch but I have a questions about it:

Why non-interactive functions of cc-mode doesn't provide doc-string?
--
Utkarsh Singh

bug#46301: 28.0.50; cc-mode: add support for c++ lambda expression

[Alan Mackenzie]

Hello, Utkarsh.

On Sat, Feb 27, 2021 at 21:17:11 +0530, Utkarsh Singh wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > I've spent the last couple of days fixing the handling of C-M-a/e in
> > cc-cmds.el.  It took rather more than the four line patch you proposed.
> > ;-)  Could I ask you, please, to apply the following patch (which should
> > apply cleanly to the Emacs master branch), byte compile cc-cmds.el, load
> > it into Emacs, and test it for me on your real source code.  Then please
> > report back whether the bug is in fact fixed, and if not, what is still
> > wrong.  In the event you would like help with the patching or byte
> > compiling, feel free to send me private email.

> Thanks now issue is fixed and code itself looks pretty solid.

Thanks for doing the testing.  I've committed the patch to all the
relevant places, and I'm closing the bug with this post.

> Due to my in-experience in Elisp and comprehensive cc-mode library I was
> unable to deep dive into the patch but I have a questions about it:

> Why non-interactive functions of cc-mode doesn't provide doc-string?

Quite a lot of functions, even non-interactive ones, and variables have
a doc string.  This means that those functions/variables are available
to modes derived from CC Mode, and their interfaces are fixed.  The ones
without doc strings are regarded as "internal" to CC Mode, and their
interfaces can (and do) change at any time.  There's a comment to this
effect burried in the first few hundred lines of cc-engine.el.

A few years ago, an alternative convention started being adopted by
Emacs, where a double hypen is used to indicate an "internal" function
or variable.

> --
> Utkarsh Singh

-- 
Alan Mackenzie (Nuremberg, Germany).