bug#13046: 23.4; Minimal Example to freeze emacs C++ mode (nasty interaction between templates & windows endline characters )

bug#13046: 23.4; Minimal Example to freeze emacs C++ mode (nasty interaction between templates & windows endline characters )

[John Smith]

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

Dear Maintainers,

Using emacs -Q,just open the attached minimal example and try typing
"std::string" in the body of the function.
That should hang emacs forever (or at least it does on my up-to-date
ubuntu).

Cheers,

[-- Attachment #1.2: Type: text/html, Size: 296 bytes --]

[-- Attachment #2: crash.hpp --]
[-- Type: text/x-c++hdr, Size: 87 bytes --]

template<>
void t() {
//type "std::string" below this line to hang emacs

    }

bug#13046: 23.4; Minimal Example to freeze emacs C++ mode (nasty interaction between templates & windows endline characters )

[Chong Yidong]

John Smith <drefleladi@gmail.com> writes:

> Using emacs -Q,just open the attached minimal example and try typing
> "std::string" in the body of the function. 
> That should hang emacs forever (or at least it does on my up-to-date
> ubuntu).

I think this is a duplicate of Bug#11841.

The patch posted in Bug#11841 does not completely fix this test case.
Giving ^M whitespace syntax in CC mode, as Stefan suggested, seems like
the easiest solution.  Alan, could you take another look?  It would be
good to have this fixed in Emacs 24.3.

bug#13046: 23.4; Minimal Example to freeze emacs C++ mode (nasty interaction between templates & windows endline characters )

[Alan Mackenzie]

Hello, Yidong.

On Sun, Dec 02, 2012 at 10:13:47AM +0800, Chong Yidong wrote:
> John Smith <drefleladi@gmail.com> writes:

> > Using emacs -Q,just open the attached minimal example and try typing
> > "std::string" in the body of the function. 
> > That should hang emacs forever (or at least it does on my up-to-date
> > ubuntu).

Yes.

> I think this is a duplicate of Bug#11841.

I think so too, though 11841 has at least two distinct bugs causing the
same symptom.

> The patch posted in Bug#11841 does not completely fix this test case.
> Giving ^M whitespace syntax in CC mode, as Stefan suggested, seems like
> the easiest solution.

The problem in John Smith's case is that c-backward-sws ("backward
syntactic whitespace") doesn't always go back over a ^M, causing the
calling routine to loop.  I'm looking into this at the moment.

> Alan, could you take another look?  It would be good to have this fixed
> in Emacs 24.3.

Agreed.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#13046: 23.4; Minimal Example to freeze emacs C++ mode (nasty interaction between templates & windows endline characters )

[Alan Mackenzie]

Hello Yidong, Vadim, John.

On Sun, Dec 02, 2012 at 10:13:47AM +0800, Chong Yidong wrote:
> John Smith <drefleladi@gmail.com> writes:

> > Using emacs -Q,just open the attached minimal example and try typing
> > "std::string" in the body of the function. 
> > That should hang emacs forever (or at least it does on my up-to-date
> > ubuntu).

> I think this is a duplicate of Bug#11841.

> The patch posted in Bug#11841 does not completely fix this test case.
> Giving ^M whitespace syntax in CC mode, as Stefan suggested, seems like
> the easiest solution.  Alan, could you take another look?  It would be
> good to have this fixed in Emacs 24.3.

On Wed, Jul 25, 2012 at 11:58:22PM +0300, Vadim K wrote:
> Hello Alan,


> I've tried your patch and it worked fine for the original posted
> bad.cpp file. However I have found a variation of that file that still
> causes emacs to hang (see attached bad2.cpp).
[ Bug #11841 ]


I think the enclosed patch fixes these bugs.  Please try out the patch
and let us know if there are still any problems here.

The basic problem is that (forward-comment -1) doesn't recognise a CR
character as whitespace.  My first attempt to patch this, in July, was
only partly successful.

The specific problem was that (forward-comment -1) _sometimes_ goes back
over the LF in CRLF, sometimes not.  I've not discovered by what
criterion.



diff -r 1adcc48506f9 cc-engine.el
--- a/cc-engine.el	Sun Apr 22 09:42:29 2012 +0000
+++ b/cc-engine.el	Sun Dec 02 17:46:53 2012 +0000
@@ -1454,8 +1454,21 @@
 	    ;; return t when moving backwards at bob.
 	    (not (bobp))
 
-	    (if (let (open-paren-in-column-0-is-defun-start)
-		  (forward-comment -1))
+	    (if (let (open-paren-in-column-0-is-defun-start moved-comment)
+		  (while
+		      (and (not (setq moved-comment (forward-comment -1)))
+		      ;; Cope specifically with ^M^J here -
+		      ;; forward-comment sometimes gets stuck after ^Ms,
+		      ;; sometimes after ^M^J.
+			   (or
+			    (when (eq (char-before) ?\r)
+			      (backward-char)
+			      t)
+			    (when (and (eq (char-before) ?\n)
+				       (eq (char-before (1- (point))) ?\r))
+			      (backward-char 2)
+			      t))))
+		  moved-comment)
 		(if (looking-at "\\*/")
 		    ;; Emacs <= 20 and XEmacs move back over the
 		    ;; closer of a block comment that lacks an opener.



-- 
Alan Mackenzie (Nuremberg, Germany).

bug#13046: 23.4; Minimal Example to freeze emacs C++ mode (nasty interaction between templates & windows endline characters )

[John Smith]

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

Hello all,

I'm afraid my limited emacs knowledge will not allow me to test the patch.
(Building fresh from source + patching the lisp => too much :) )
I trust however that it does!

Thanks for being so responsive.

Yours Sincerely,
John

On Sun, Dec 2, 2012 at 7:16 PM, Alan Mackenzie <acm@muc.de> wrote:

> Hello Yidong, Vadim, John.
>
> On Sun, Dec 02, 2012 at 10:13:47AM +0800, Chong Yidong wrote:
> > John Smith <drefleladi@gmail.com> writes:
>
> > > Using emacs -Q,just open the attached minimal example and try typing
> > > "std::string" in the body of the function.
> > > That should hang emacs forever (or at least it does on my up-to-date
> > > ubuntu).
>
> > I think this is a duplicate of Bug#11841.
>
> > The patch posted in Bug#11841 does not completely fix this test case.
> > Giving ^M whitespace syntax in CC mode, as Stefan suggested, seems like
> > the easiest solution.  Alan, could you take another look?  It would be
> > good to have this fixed in Emacs 24.3.
>
> On Wed, Jul 25, 2012 at 11:58:22PM +0300, Vadim K wrote:
> > Hello Alan,
>
>
> > I've tried your patch and it worked fine for the original posted
> > bad.cpp file. However I have found a variation of that file that still
> > causes emacs to hang (see attached bad2.cpp).
> [ Bug #11841 ]
>
>
> I think the enclosed patch fixes these bugs.  Please try out the patch
> and let us know if there are still any problems here.
>
> The basic problem is that (forward-comment -1) doesn't recognise a CR
> character as whitespace.  My first attempt to patch this, in July, was
> only partly successful.
>
> The specific problem was that (forward-comment -1) _sometimes_ goes back
> over the LF in CRLF, sometimes not.  I've not discovered by what
> criterion.
>
>
>
> diff -r 1adcc48506f9 cc-engine.el
> --- a/cc-engine.el      Sun Apr 22 09:42:29 2012 +0000
> +++ b/cc-engine.el      Sun Dec 02 17:46:53 2012 +0000
> @@ -1454,8 +1454,21 @@
>             ;; return t when moving backwards at bob.
>             (not (bobp))
>
> -           (if (let (open-paren-in-column-0-is-defun-start)
> -                 (forward-comment -1))
> +           (if (let (open-paren-in-column-0-is-defun-start moved-comment)
> +                 (while
> +                     (and (not (setq moved-comment (forward-comment -1)))
> +                     ;; Cope specifically with ^M^J here -
> +                     ;; forward-comment sometimes gets stuck after ^Ms,
> +                     ;; sometimes after ^M^J.
> +                          (or
> +                           (when (eq (char-before) ?\r)
> +                             (backward-char)
> +                             t)
> +                           (when (and (eq (char-before) ?\n)
> +                                      (eq (char-before (1- (point))) ?\r))
> +                             (backward-char 2)
> +                             t))))
> +                 moved-comment)
>                 (if (looking-at "\\*/")
>                     ;; Emacs <= 20 and XEmacs move back over the
>                     ;; closer of a block comment that lacks an opener.
>
>
>
> --
> Alan Mackenzie (Nuremberg, Germany).
>

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

bug#13046: 23.4; Minimal Example to freeze emacs C++ mode (nasty interaction between templates & windows endline characters )

[Alan Mackenzie]

Hello, John.

On Mon, Dec 03, 2012 at 09:54:51AM +0100, John Smith wrote:
> Hello all,

> I'm afraid my limited emacs knowledge will not allow me to test the patch.
> (Building fresh from source + patching the lisp => too much :) )
> I trust however that it does!

If you want to test the patch, here's how to go about it:
(i) Make sure you've got the program `patch' installed - it's in package
"diffutils".  (I don't know the package manager in Ubuntu).

(ii) Extract my email containing the patch to a file, say foo.diff.  You
don't need to bother separating out the actual patch from all the other
stuff.

(iii) Locate cc-engine.el.  It will be somewhere like
/usr/local/share/emacs/24.2/lisp/progmodes/cc-engine.el.gz if you
installed Emacs from source, or
/usr/share/emacs/24.2/lisp/progmodes/cc-engine.el.gz if you installed
Emacs from a package manager.  In either case you can find it for sure with
C-h f c-mode <ret>, and click/type CR on the file name.  Then do C-x C-v
to see where it is.

(iv) Unzip the source file: cd <above directory>; gunzip cc-engine.el.gz

(v) Patch that file: patch < /path/to/foo.diff

(vi) Byte compile the file: (in emacs)
M-x byte-compile-file <ret> /path/to/cc-engine.el

(vii) Load the file (or restart Emacs):
M-x load-file /path/to/cc-engine.elc
(note the 'c' at the end of ".elc")

(viii) Advisable: gunzip, byte compile, and load cc-cmds.el.gz too.
This is because it uses the "defsubst" (inline function) changed in
cc-engine.el.

> Thanks for being so responsive.

Thank you for such a high quality bug report.

> Yours Sincerely,
> John

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#13046: 23.4; Minimal Example to freeze emacs C++ mode (nasty interaction between templates & windows endline characters )

[John Smith]

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

Damn you.
I *told* you that I didn't have the time & skills to look into this further
and you being so-nice-and-all guilt forced me into doing it :)

ok, so a few take-aways from giving advices to an emacs noob:

-on ubuntu the distribution only packages compiled lisp files so one needs
to add a special package to get the sources

-*In either case you can find it for sure with C-h f c-mode <ret>, and
click/type CR on the file name.  Then do C-x C-v to see where it is.*
I could switch to the C-mode but never understood what "*click/type CR on
the file name"* was supposed to mean. Anyways I found the file.

-the patch didn't work since the lines you removed changed a bit between
23.4 and HEAD, but I worked around it.

-like all good things there are dependencies: cc-defs.el for one and
friends, I had to fetch them

-mmh, it doesn't byte compile. actually, even vanilla unpatched file
doesn't byte-compile:
In c-declare-lang-variables:
cc-engine.el:162:28:Warning: reference to free variable
    `c-lang-variable-inits'
cc-engine.el:163:1:Error: Symbol's value as variable is void:
c-lang-variable-inits

I tried commenting out that line but no such luck.
I guess this is a trivial problem and I need to build something else but
mmh what? I'm stuck.
Years of devs in heaps of languages and yet I'm so helpless with this one.

Cheers and thanks for the trip.
John



On Wed, Dec 5, 2012 at 9:10 PM, Alan Mackenzie <acm@muc.de> wrote:

> Hello, John.
>
> On Mon, Dec 03, 2012 at 09:54:51AM +0100, John Smith wrote:
> > Hello all,
>
> > I'm afraid my limited emacs knowledge will not allow me to test the
> patch.
> > (Building fresh from source + patching the lisp => too much :) )
> > I trust however that it does!
>
> If you want to test the patch, here's how to go about it:
> (i) Make sure you've got the program `patch' installed - it's in package
> "diffutils".  (I don't know the package manager in Ubuntu).
>
> (ii) Extract my email containing the patch to a file, say foo.diff.  You
> don't need to bother separating out the actual patch from all the other
> stuff.
>
> (iii) Locate cc-engine.el.  It will be somewhere like
> /usr/local/share/emacs/24.2/lisp/progmodes/cc-engine.el.gz if you
> installed Emacs from source, or
> /usr/share/emacs/24.2/lisp/progmodes/cc-engine.el.gz if you installed
> Emacs from a package manager.  In either case you can find it for sure with
> C-h f c-mode <ret>, and click/type CR on the file name.  Then do C-x C-v
> to see where it is.
>
> (iv) Unzip the source file: cd <above directory>; gunzip cc-engine.el.gz
>
> (v) Patch that file: patch < /path/to/foo.diff
>
> (vi) Byte compile the file: (in emacs)
> M-x byte-compile-file <ret> /path/to/cc-engine.el
>
> (vii) Load the file (or restart Emacs):
> M-x load-file /path/to/cc-engine.elc
> (note the 'c' at the end of ".elc")
>
> (viii) Advisable: gunzip, byte compile, and load cc-cmds.el.gz too.
> This is because it uses the "defsubst" (inline function) changed in
> cc-engine.el.
>
> > Thanks for being so responsive.
>
> Thank you for such a high quality bug report.
>
> > Yours Sincerely,
> > John
>
> --
> Alan Mackenzie (Nuremberg, Germany).
>

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

bug#13046: 23.4; Minimal Example to freeze emacs C++ mode (nasty interaction between templates & windows endline characters )

[Alan Mackenzie]

On Fri, Dec 07, 2012 at 05:34:32AM +0100, John Smith wrote:
> Damn you.
> I *told* you that I didn't have the time & skills to look into this further
> and you being so-nice-and-all guilt forced me into doing it :)

> ok, so a few take-aways from giving advices to an emacs noob:

> -on ubuntu the distribution only packages compiled lisp files so one needs
> to add a special package to get the sources

> -*In either case you can find it for sure with C-h f c-mode <ret>, and
> click/type CR on the file name.  Then do C-x C-v to see where it is.*
> I could switch to the C-mode but never understood what "*click/type CR on
> the file name"* was supposed to mean. Anyways I found the file.

> -the patch didn't work since the lines you removed changed a bit between
> 23.4 and HEAD, but I worked around it.

> -like all good things there are dependencies: cc-defs.el for one and
> friends, I had to fetch them

Sorry, I'd forgotten about the dependencies.

> -mmh, it doesn't byte compile. actually, even vanilla unpatched file
> doesn't byte-compile:
> In c-declare-lang-variables:
> cc-engine.el:162:28:Warning: reference to free variable
>     `c-lang-variable-inits'
> cc-engine.el:163:1:Error: Symbol's value as variable is void:
> c-lang-variable-inits

> I tried commenting out that line but no such luck.
> I guess this is a trivial problem and I need to build something else but
> mmh what? I'm stuck.
> Years of devs in heaps of languages and yet I'm so helpless with this one.

c-lang-variable-inits is needed at compile time, and is in a different
file.  It would probably be easiest at this stage just to compile the
entire CC Mode.  To do this, get all the files cc-*.el (there are 17)
gunzipped, and give this at the command line (not within Emacs):

$ emacs -Q -batch -f batch-byte-compile cc-*.el

> Cheers and thanks for the trip.

:-)

> John

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#13046: 23.4; Minimal Example to freeze emacs C++ mode (nasty interaction between templates & windows endline characters )

[John Smith]

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

Hello,

I confirm that the patch indeed works for my work-file (which is a tad more
complicated than my contrived example)

Thanks,
Fred

On Sun, Dec 9, 2012 at 5:58 PM, Alan Mackenzie <acm@muc.de> wrote:

> On Fri, Dec 07, 2012 at 05:34:32AM +0100, John Smith wrote:
> > Damn you.
> > I *told* you that I didn't have the time & skills to look into this
> further
> > and you being so-nice-and-all guilt forced me into doing it :)
>
> > ok, so a few take-aways from giving advices to an emacs noob:
>
> > -on ubuntu the distribution only packages compiled lisp files so one
> needs
> > to add a special package to get the sources
>
> > -*In either case you can find it for sure with C-h f c-mode <ret>, and
> > click/type CR on the file name.  Then do C-x C-v to see where it is.*
> > I could switch to the C-mode but never understood what "*click/type CR on
> > the file name"* was supposed to mean. Anyways I found the file.
>
> > -the patch didn't work since the lines you removed changed a bit between
> > 23.4 and HEAD, but I worked around it.
>
> > -like all good things there are dependencies: cc-defs.el for one and
> > friends, I had to fetch them
>
> Sorry, I'd forgotten about the dependencies.
>
> > -mmh, it doesn't byte compile. actually, even vanilla unpatched file
> > doesn't byte-compile:
> > In c-declare-lang-variables:
> > cc-engine.el:162:28:Warning: reference to free variable
> >     `c-lang-variable-inits'
> > cc-engine.el:163:1:Error: Symbol's value as variable is void:
> > c-lang-variable-inits
>
> > I tried commenting out that line but no such luck.
> > I guess this is a trivial problem and I need to build something else but
> > mmh what? I'm stuck.
> > Years of devs in heaps of languages and yet I'm so helpless with this
> one.
>
> c-lang-variable-inits is needed at compile time, and is in a different
> file.  It would probably be easiest at this stage just to compile the
> entire CC Mode.  To do this, get all the files cc-*.el (there are 17)
> gunzipped, and give this at the command line (not within Emacs):
>
> $ emacs -Q -batch -f batch-byte-compile cc-*.el
>
> > Cheers and thanks for the trip.
>
> :-)
>
> > John
>
> --
> Alan Mackenzie (Nuremberg, Germany).
>

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

bug#13046: 23.4; Minimal Example to freeze emacs C++ mode (nasty interaction between templates & windows endline characters )

[Chong Yidong]

Alan Mackenzie <acm@muc.de> writes:

> I think the enclosed patch fixes these bugs.  Please try out the patch
> and let us know if there are still any problems here.

Seems to work OK.  Looks like a bit of a hack, but if it fixes the bug,
so be it.  Feel free to commit to emacs-24 if you're happy with it, and
thanks.

bug#13046: 23.4; Minimal Example to freeze emacs C++ mode (nasty interaction between templates & windows endline characters )

[Vadim K]

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

Hi Alan,


I've verified that this patch does fix the issue #11841. On my Windows XP,
after applying this patch, emacs was able to open both files that caused
the issue.


Vadim

On Sun, Dec 2, 2012 at 8:16 PM, Alan Mackenzie <acm@muc.de> wrote:

>
> I think the enclosed patch fixes these bugs.  Please try out the patch
> and let us know if there are still any problems here.
>
>  --
> Alan Mackenzie (Nuremberg, Germany).
>

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