owenpan added a comment.
In D95168#3038531 <https://reviews.llvm.org/D95168#3038531>, @MyDeveloperDay
wrote:
> @tiagoma are you still interested in pursuing this? I have some suggestions
>
> 1. I'd like to move the BraceInserter Into its own .cpp and .h files (like I
> did with the QualifierAlignmentFixer)
> 2. I'd like to move the unit tests into their own .cpp file (because I think
> we need to actually unit tests their functions of BraceInserter more than
> just testing if via verfiyFormat and I think its cleaner as FormatTest.cpp is
> very large)
> 3. I'd like to see what it would take to remove braces, (eliding the braces
> on small ifs and control statements is about the number one review comments
> in LLVM)
Eliding braces would be much more complicated and should be tackled separately.
Below are just some examples:
void f() {
// Braces can be removed:
if (a) {
b;
} else if (c) {
d;
} else {
e;
}
if (a) {
b;
c;
} else if (d) { // Braces may be removed depending on the style.
#undef NDEBUG
e;
} else if (g) {
// comment
} else {
{
h;
}
}
if (a) {
if (b) { // Braces can be removed.
c;
}
} else if (d) { // Braces may be removed depending on the style.
e;
}
if (a) { // Braces can be removed if the dangling-else warning can be
ignored.
if (b) { // Braces may be removed depending on the style.
c;
// comment
} else if (d) { // Braces may be removed depending on the style.
e;
}
}
// Braces can be removed:
if (a) {
if (b) {
c;
}
}
// Braces may be removed depending on the style:
if (a) {
if (b) {
c;
} else {
d;
}
} else {
e;
}
if (a) { // Braces may be removed depending on the style.
// Braces can be removed:
if (b) {
c;
} else if (d) {
e;
}
} else { // Braces may be removed depending on the style.
g;
}
// Braces can be removed:
if (a) {
b;
} else {
if (c) {
d;
} else {
e;
}
}
I have an experimental implementation that reformats the above to the following
(avoiding the dangling-else warning, matching braces of `if` and `else`, etc.):
void f() {
// Braces can be removed:
if (a)
b;
else if (c)
d;
else
e;
if (a) {
b;
c;
} else if (d) { // Braces may be removed depending on the style.
#undef NDEBUG
e;
} else if (g) {
// comment
} else {
{ h; }
}
if (a) {
if (b) // Braces can be removed.
c;
} else if (d) { // Braces may be removed depending on the style.
e;
}
if (a) { // Braces can be removed if the dangling-else warning can be
ignored.
if (b) { // Braces may be removed depending on the style.
c;
// comment
} else if (d) { // Braces may be removed depending on the style.
e;
}
}
// Braces can be removed:
if (a)
if (b)
c;
// Braces may be removed depending on the style:
if (a)
if (b)
c;
else
d;
else
e;
if (a) { // Braces may be removed depending on the style.
// Braces can be removed:
if (b)
c;
else if (d)
e;
} else { // Braces may be removed depending on the style.
g;
}
// Braces can be removed:
if (a)
b;
else if (c)
d;
else
e;
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95168/new/
https://reviews.llvm.org/D95168
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
