Skip to content
This repository has been archived by the owner. It is now read-only.

Inconsistent behavior regarding line break before access modifier. #41870

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

Inconsistent behavior regarding line break before access modifier. #41870

llvmbot opened this issue May 14, 2019 · 4 comments

Comments

@llvmbot
Copy link
Collaborator

@llvmbot 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: #16518

#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 llvmbot commented May 15, 2019

OS: Win10
Clang version: 8.0

Loading

@MaxSagebaum
Copy link
Mannequin

@MaxSagebaum MaxSagebaum mannequin commented Mar 29, 2021

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.

Loading

@MaxSagebaum
Copy link
Mannequin

@MaxSagebaum MaxSagebaum mannequin commented Mar 29, 2021

Created a merge request for the fix.

https://reviews.llvm.org/D99503

Loading

@MaxSagebaum
Copy link
Mannequin

@MaxSagebaum MaxSagebaum mannequin commented Apr 21, 2021

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

Loading

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant