Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent behavior regarding line break before access modifier. #41215

Closed
llvmbot opened this issue May 14, 2019 · 4 comments
Closed

Inconsistent behavior regarding line break before access modifier. #41215

llvmbot opened this issue May 14, 2019 · 4 comments
Labels
bugzilla Issues migrated from bugzilla clang-tools-extra

Comments

@llvmbot
Copy link
Collaborator

llvmbot commented May 14, 2019

Bugzilla Link 41870
Resolution FIXED
Resolved on Apr 21, 2021 00:51
Version unspecified
OS other
Attachments Files to reproduce bug
Reporter LLVM Bugzilla Contributor
CC @MaxSagebaum

Extended Description

#Description

Inconsistent behavior regarding line break before access modifier.

If a newline exists it gets removed. If no newline exists it gets added.

Similar but not quite the same: #16892

#How to reproduce:
clang-format --style=file example.hpp > example_after.hpp

##.clang-format
MaxEmptyLinesToKeep: 0

##example.hpp
class a {
public:
void b();
public: // newline gets inserted
void c();

public: // newline gets removed
void d();
}

##example_after.hpp
class a {
public:
void b();

public: // newline gets inserted
void c();
public: // newline gets removed
void d();
}

@llvmbot
Copy link
Collaborator Author

llvmbot commented May 15, 2019

OS: Win10
Clang version: 8.0

@MaxSagebaum
Copy link
Contributor

Possible bugfix
Reproduces on
OS: Fedora
Clang version: 11.0 and 12.0

The problem is that, Newlines is set to 1 because of the setting MaxEmptyLinesToKeep. The checks in the logic are based on RootToken.NewlinesBefore which is 2 and therefore Newlines is not changes. The checks need to be changed to the actual value of Newlines.

I will submit a merge request for the change.

@MaxSagebaum
Copy link
Contributor

Created a merge request for the fix.

https://reviews.llvm.org/D99503

@MaxSagebaum
Copy link
Contributor

Fixed via merge request https://reviews.llvm.org/D99503.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang-tools-extra
Projects
None yet
Development

No branches or pull requests

2 participants