LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 41413 - clang-format incorrectly indents wrapped closing parenthesis
Summary: clang-format incorrectly indents wrapped closing parenthesis
Status: RESOLVED FIXED
Alias: None
Product: clang
Classification: Unclassified
Component: Formatter (show other bugs)
Version: trunk
Hardware: All All
: P normal
Assignee: Owen Pan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-04-06 22:53 PDT by Owen Pan
Modified: 2019-04-07 14:35 PDT (History)
5 users (show)

See Also:
Fixed By Commit(s): r357877


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Pan 2019-04-06 22:53:40 PDT
The wrapped closing parentheses below should not be but are indented:

Foo::Foo(
    //
)
    : foo(0) {}

int Foo::getter(
    //
) const {
  return foo;
}
Comment 1 Owen Pan 2019-04-06 23:04:45 PDT
See https://reviews.llvm.org/D60374
Comment 2 MyDeveloperDay 2019-04-07 05:36:18 PDT
Your saying the ')' should not be aligned to the // is that correct?

Foo::Foo(
    //
    )
    : foo(0) {}

int Foo::getter(
    //
    ) const {
  return foo;
}
Comment 3 Manuel Klimek 2019-04-07 08:00:13 PDT
I'd say the current behavior is more readable.
Comment 4 Owen Pan 2019-04-07 08:46:06 PDT
The current behavior is inconsistent: the closing parenthesis is not indented if not followed by colon or const.
Comment 5 MyDeveloperDay 2019-04-07 09:01:32 PDT
Owen, Sorry I apologize if I'm not getting it straight away, but maybe I see it now so can you confirm,  am I right in thinking what you are saying is 

If I have a function like bar() here, the the ) will be left aligned

Foo::bar(
    //
) {}

but if I have : or const it will be aligned with the //

Foo::Foo(
    //
    )
    : foo(0) {}

int Foo::getter(
    //
    ) const {
  return foo;
}

Your fix is to always left align the ) in such circumstances so its consistent

I think I agree with you that these 2 should be consistent

void Foo::bar( // some comment
) {}

void Foo::bar( // some comment
) const {}


Given that they currently produce the following seems odd

void Foo::bar( // some comment
) {}

void Foo::bar( // some comment
    ) const {}
Comment 6 Owen Pan 2019-04-07 12:09:04 PDT
(In reply to MyDeveloperDay from comment #2)
> Your saying the ')' should not be aligned to the // is that correct?
> 
> Foo::Foo(
>     //
>     )
>     : foo(0) {}
> 
> int Foo::getter(
>     //
>     ) const {
>   return foo;
> }

Yes, and the bug lies in the inconsistency, at least in the case of const.
Comment 7 Owen Pan 2019-04-07 12:13:50 PDT
(In reply to MyDeveloperDay from comment #5)
> Owen, Sorry I apologize if I'm not getting it straight away, but maybe I see
> it now so can you confirm,  am I right in thinking what you are saying is 
> 
> If I have a function like bar() here, the the ) will be left aligned
> 
> Foo::bar(
>     //
> ) {}
> 
> but if I have : or const it will be aligned with the //
> 
> Foo::Foo(
>     //
>     )
>     : foo(0) {}
> 
> int Foo::getter(
>     //
>     ) const {
>   return foo;
> }
> 
> Your fix is to always left align the ) in such circumstances so its
> consistent

Yes. :)

> I think I agree with you that these 2 should be consistent
> 
> void Foo::bar( // some comment
> ) {}
> 
> void Foo::bar( // some comment
> ) const {}
> 
> 
> Given that they currently produce the following seems odd
> 
> void Foo::bar( // some comment
> ) {}
> 
> void Foo::bar( // some comment
>     ) const {}

It's clear to me that we should fix it in the case of const but maybe should leave the colon case alone?