[clang-format][NFC] Simplify some logic in BreakableLineCommentSection

[owenca]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/148324.diff

1 Files Affected:

• (modified) clang/lib/Format/BreakableToken.cpp (+4-6)

diff --git a/clang/lib/Format/BreakableToken.cpp b/clang/lib/Format/BreakableToken.cpp
index 24912c25ef8c6..c36cb74bc4501 100644
--- a/clang/lib/Format/BreakableToken.cpp
+++ b/clang/lib/Format/BreakableToken.cpp
@@ -927,14 +927,12 @@ BreakableLineCommentSection::BreakableLineCommentSection(
       }
 
       if (Lines[i].size() != IndentPrefix.size()) {
-        PrefixSpaceChange[i] = FirstLineSpaceChange;
+        assert(Lines[i].size() > IndentPrefix.size());
 
-        if (SpacesInPrefix + PrefixSpaceChange[i] < Minimum) {
-          PrefixSpaceChange[i] +=
-              Minimum - (SpacesInPrefix + PrefixSpaceChange[i]);
-        }
+        PrefixSpaceChange[i] = SpacesInPrefix + FirstLineSpaceChange < Minimum
+                                   ? Minimum - SpacesInPrefix
+                                   : FirstLineSpaceChange;
 
-        assert(Lines[i].size() > IndentPrefix.size());
         const auto FirstNonSpace = Lines[i][IndentPrefix.size()];
         const bool IsFormatComment = LineTok && switchesFormatting(*LineTok);
         const bool LineRequiresLeadingSpace =

[HazardyKnusperkeks]

Suggested change

	PrefixSpaceChange[i] = SpacesInPrefix + FirstLineSpaceChange < Minimum
	PrefixSpaceChange[i] = (SpacesInPrefix + FirstLineSpaceChange) < Minimum

This is what you wanted, or?

[owenca]

Why would we want to add the redundant parentheses? They were not there in the if statement before.

[HazardyKnusperkeks]

I was thinking wrong here. But adding parentheses when using the ternary operator helps reading it. I thought that SpaceInPrefix was always the base.

[owenca]

I see, but in general, how could var1 + var2 < var3 be misread as var1 + (var2 < var3), whether the former is in if/while, ternary, or assignment? In fact, I would always remove the redundant parentheses in (var1 + var2) < var3.

[HazardyKnusperkeks]

Yeah, I was mistaken.

But, even is not needed, I'd do this:

Suggested change

	PrefixSpaceChange[i] = SpacesInPrefix + FirstLineSpaceChange < Minimum
	PrefixSpaceChange[i] = (SpacesInPrefix + FirstLineSpaceChange < Minimum)

[owenca]

Like the examples of conditional/ternary operators, I never add redundant parentheses because they have the lowest precedence relative to arithmetic/logical operators. This is analogous to a + (b * c) or (a / b) - c.

The clang-format codebase has a mixture of both expr1 ? expr2 : expr3 and (expr1) ? expr2 : expr3, and IMO we should consistently use the former. Although I can understand why some people prefer the latter when expr1 and expr2 are on the same line, there's really nothing to gain when the questions mark starts on a new line:

expr1
    ? expr2

[HazardyKnusperkeks]

Yeah, I was mistaken.

But, even is not needed, I'd do this:

Suggested change

	PrefixSpaceChange[i] = SpacesInPrefix + FirstLineSpaceChange < Minimum
	PrefixSpaceChange[i] = (SpacesInPrefix + FirstLineSpaceChange < Minimum)