[Bug] Indentation of C++ constructor with initializer list

[James McCoy]

Given the file foo.cc

-- >8 --
class a {
public:
a() : i(0)
{
}

a()
: i(0)
{
}

a() : i(0) {
}
};
-- 8< --

Performing '=G' from line 1 results in

-- >8 --
class a {
public:
a() : i(0)
{
}

a()
: i(0)
{
}

a() : i(0) {
}
};
-- 8< --

The block of the constructor on line 3 gets dedented when it shouldn't.
The backtracking to find the start of the constructor, so it can be used
as the basis for indenting the block, finds the scope declaration and
keys off of that instead.

The cindent code is pretty hairy. I wasn't able to find an obvious fix.

Cheers,
--
James
GPG Key: 4096R/331BA3DB 2011-12-05 James McCoy <jame...@jamessan.com>

[h_east]

Hi James!

2015-8-2 (Sun) 13:46:34 UTC+9 James McCoy:

> Given the file foo.cc
>
> -- >8 --
> class a {
> public:
> a() : i(0)
> {
> }
>
> a()
> : i(0)
> {
> }
>
> a() : i(0) {
> }
> };
> -- 8< --
>
> Performing '=G' from line 1 results in
>
> -- >8 --
> class a {
> public:
> a() : i(0)
> {
> }
>
> a()
> : i(0)
> {
> }
>
> a() : i(0) {
> }
> };
> -- 8< --
>
> The block of the constructor on line 3 gets dedented when it shouldn't.
> The backtracking to find the start of the constructor, so it can be used
> as the basis for indenting the block, finds the scope declaration and
> keys off of that instead.
>
> The cindent code is pretty hairy. I wasn't able to find an obvious fix.

I can reproduce it.
And began to investigate.
Perhaps I would fix this problem.

Please wait a week.

--
Best regards,
Hirohito Higashi (a.k.a h_east)

[h_east]

Hi James and Bram,

2015-10-4 (Sun) 22:29:23 UTC+9 h_east:

I got it!

Please confirm an attached patch. (Contains test)

[h_east]

Hi

2015-10-7 (Wed) 10:26:08 UTC+9 h_east:

Oops, this patch makes another problem.

The following Issue will further badly.
https://github.com/vim/vim/issues/38

#v+
void func()
{
switch (foo)
{
case (bar):
if (baz())
quux(); // FIXME: this line should be indented more!
break;
case (shmoo):
if (!bar)
{ // FIXME: this brace is indented too little
}
case (foo1):
switch (bar)
{ // FIXME: this brace is indented too little
case baz:
baz_f();
break;
}
break;
default:
baz();
baz();
break;
}
}
#v-

I will investigate next weekend.

[h_east]

Hi Bram,

2015-10-12 (Mon) 0:18:12 UTC+9 h_east:

Bram>
Thanks for fix above problem.
Patch 7.4.893
https://groups.google.com/d/msg/vim_dev/9-mLe9urjeg/NJqaLAe-CgAJ

I update this issue's patch.
I confirmed test results is ALL DONE.

[James McCoy]

On Tue, Oct 13, 2015 at 05:24:19PM -0700, h_east wrote:
> Bram>
> Thanks for fix above problem.
> Patch 7.4.893
> https://groups.google.com/d/msg/vim_dev/9-mLe9urjeg/NJqaLAe-CgAJ
>
> I update this issue's patch.
> I confirmed test results is ALL DONE.

It looks like this was never included and I don't see mention of it in
the todo list.

> diff -r 30042ddff503 src/misc1.c
> --- a/src/misc1.c Tue Oct 13 23:30:05 2015 +0200
> +++ b/src/misc1.c Wed Oct 14 09:14:22 2015 +0900
> @@ -5496,8 +5496,9 @@
>
> /* Find result cache for cpp_baseclass */
> typedef struct {
> - int found;
> - lpos_T lpos;
> + int found;
> + linenr_T start_of_decl;
> + lpos_T lpos;
> } cpp_baseclass_cache_T;
>
> /*
> @@ -6505,6 +6506,8 @@
> int class_or_struct, lookfor_ctor_init, cpp_base_class;
> linenr_T lnum = curwin->w_cursor.lnum;
> char_u *line = ml_get_curline();
> + linenr_T start_of_decl;
> + pos_T end_paren = { 0, 0 };
>
> if (pos->lnum <= lnum)
> return cached->found; /* Use the cached result */
> @@ -6553,6 +6556,7 @@
> --lnum;
> }
>
> + start_of_decl = lnum;
> pos->lnum = lnum;
> line = ml_get(lnum);
> s = line;
> @@ -6604,6 +6608,8 @@
> {
> class_or_struct = TRUE;
> lookfor_ctor_init = FALSE;
> + if (!cpp_base_class)
> + start_of_decl = lnum;
>
> if (*s == 'c')
> s = cin_skipcomment(s + 5);
> @@ -6622,6 +6628,12 @@
> * something like "):" */
> class_or_struct = FALSE;
> lookfor_ctor_init = TRUE;
> + if (!cpp_base_class)
> + {
> + start_of_decl = 0;
> + end_paren.lnum = lnum;
> + end_paren.col = (colnr_T)(s - line);
> + }
> }
> else if (s[0] == '?')
> {
> @@ -6654,7 +6666,28 @@
>
> cached->found = cpp_base_class;
> if (cpp_base_class)
> + {
> pos->lnum = lnum;
> + if (start_of_decl > 0)
> + cached->start_of_decl = start_of_decl;
> + else
> + {
> + pos_T *start_paren;
> + pos_T cursor_save = curwin->w_cursor;
> +
> + curwin->w_cursor = end_paren;
> + start_paren = findmatchlimit(NULL, '(', FM_BACKWARD,
> + curbuf->b_ind_maxparen);
> + curwin->w_cursor = cursor_save;
> + if (start_paren == NULL)
> + cached->start_of_decl = end_paren.lnum;
> + else
> + cached->start_of_decl = start_paren->lnum;
> + }
> + }
> + else
> + cached->start_of_decl = 0;
> +
> return cpp_base_class;
> }
>
> @@ -7204,7 +7237,7 @@
> int original_line_islabel;
> int added_to_amount = 0;
> int js_cur_has_key = 0;
> - cpp_baseclass_cache_T cache_cpp_baseclass = { FALSE, { MAXLNUM, 0 } };
> + cpp_baseclass_cache_T cache_cpp_baseclass = { FALSE, 0, { MAXLNUM, 0 } };
>
> /* make a copy, value is changed below */
> int ind_continuation = curbuf->b_ind_continuation;
> @@ -8273,6 +8306,16 @@
> }
> else if (theline[0] == '{')
> {
> + /*
> + * Get indent and pointer to text for current line,
> + * ignoring any jump label. XXX
> + */
> + curwin->w_cursor.lnum
> + = cache_cpp_baseclass.start_of_decl;
> + if (curbuf->b_ind_js)
> + amount = get_indent();
> + else
> + amount = skip_label(curwin->w_cursor.lnum, &l);
> /* Need to find start of the declaration. */
> lookfor = LOOKFOR_UNTERM;
> ind_continuation = 0;
> diff -r 30042ddff503 src/testdir/test3.in
> --- a/src/testdir/test3.in Tue Oct 13 23:30:05 2015 +0200
> +++ b/src/testdir/test3.in Wed Oct 14 09:14:22 2015 +0900
> @@ -959,6 +959,29 @@
> }
> }
>
> +class a {
> + public:
> + a()
> + : i(0)
> + {
> + }
> +
> + a() : i(0)
> + {
> + }
> +
> + Constructor(
> + int a,
> + int b,
> + int c
> + ) : base(0)
> + {
> + }
> +
> + a() : i(0) {
> + }
> +};
> +
> /* end of AUTO */
>
> STARTTEST
> diff -r 30042ddff503 src/testdir/test3.ok
> --- a/src/testdir/test3.ok Tue Oct 13 23:30:05 2015 +0200
> +++ b/src/testdir/test3.ok Wed Oct 14 09:14:22 2015 +0900
> @@ -947,6 +947,29 @@
> }
> }
>
> +class a {
> + public:
> + a()
> + : i(0)
> + {
> + }
> +
> + a() : i(0)
> + {
> + }
> +
> + Constructor(
> + int a,
> + int b,
> + int c
> + ) : base(0)
> + {
> + }
> +
> + a() : i(0) {
> + }
> +};
> +
> /* end of AUTO */

[h_east]

Hi James,

2016-3-31(Thu) 13:16:30 UTC+9 James McCoy:

> On Tue, Oct 13, 2015 at 05:24:19PM -0700, h_east wrote:
> > Bram>
> > Thanks for fix above problem.
> > Patch 7.4.893
> > https://groups.google.com/d/msg/vim_dev/9-mLe9urjeg/NJqaLAe-CgAJ
> >
> > I update this issue's patch.
> > I confirmed test results is ALL DONE.
>
> It looks like this was never included and I don't see mention of it in
> the todo list.

[...]

Yeah, Thank you for reminding me about this :-)
I update a patch.

--
Best regards,
Hirohito Higashi (a.k.a. h_east)

[Bram Moolenaar]

Hirohito Higashi wrote:

> 2016-3-31(Thu) 13:16:30 UTC+9 James McCoy:
> > On Tue, Oct 13, 2015 at 05:24:19PM -0700, h_east wrote:
> > > Bram>
> > > Thanks for fix above problem.
> > > Patch 7.4.893
> > > https://groups.google.com/d/msg/vim_dev/9-mLe9urjeg/NJqaLAe-CgAJ
> > >
> > > I update this issue's patch.
> > > I confirmed test results is ALL DONE.
> >
> > It looks like this was never included and I don't see mention of it in
> > the todo list.
> [...]
>
> Yeah, Thank you for reminding me about this :-)
> I update a patch.

Thanks. I can't find it in the todo list, I'll add it now.

--
hundred-and-one symptoms of being an internet addict:
176. You lie, even to user-friends, about how long you were online yesterday.

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///