bdunkin wrote:
@owenca just a friendly bump on this review. The main question is currently
whether changing tests is acceptable.
https://github.com/llvm/llvm-project/pull/143781
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llv
@@ -1318,281 +1343,249 @@ void
WhitespaceManager::alignArrayInitializers(unsigned Start, unsigned End) {
void WhitespaceManager::alignArrayInitializersRightJustified(
CellDescriptions &&CellDescs) {
- if (!CellDescs.isRectangular())
+
+ // If there are less than two row
@@ -1609,6 +1602,10 @@ WhitespaceManager::linkCells(CellDescriptions
&&CellDesc) {
return std::move(CellDesc);
}
+void WhitespaceManager::setChangeSpaces(unsigned Start, unsigned Spaces) {
bdunkin wrote:
The member function is a convenience for using the n
@@ -67,11 +67,11 @@ void WhitespaceManager::addUntouchableToken(const
FormatToken &Tok,
bool InPPDirective) {
if (Tok.Finalized || (Tok.MacroCtx && Tok.MacroCtx->Role == MR_ExpandedArg))
return;
- Changes.push_back(Change(Tok,
https://github.com/bdunkin edited
https://github.com/llvm/llvm-project/pull/143781
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -118,9 +118,9 @@ const tooling::Replacements
&WhitespaceManager::generateReplacements() {
alignConsecutiveTableGenDefinitions();
}
alignChainedConditionals();
+ alignArrayInitializers();
bdunkin wrote:
Aligning elements in arrays can changed the l
@@ -274,76 +268,17 @@ class WhitespaceManager {
void alignArrayInitializersLeftJustified(CellDescriptions &&CellDescs);
/// Calculate the cell width between two indexes.
- unsigned calculateCellWidth(unsigned Start, unsigned End,
- bool WithSp
https://github.com/bdunkin updated
https://github.com/llvm/llvm-project/pull/143781
Unicorn! ยท GitHub
body {
background-color: #f1f1f1;
margin: 0;
font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;
}
.container { margin: 50px au
https://github.com/bdunkin edited
https://github.com/llvm/llvm-project/pull/143781
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1318,281 +1343,253 @@ void
WhitespaceManager::alignArrayInitializers(unsigned Start, unsigned End) {
void WhitespaceManager::alignArrayInitializersRightJustified(
CellDescriptions &&CellDescs) {
- if (!CellDescs.isRectangular())
+
+ // If there are less than two row
@@ -1318,281 +1343,253 @@ void
WhitespaceManager::alignArrayInitializers(unsigned Start, unsigned End) {
void WhitespaceManager::alignArrayInitializersRightJustified(
bdunkin wrote:
This function, and the left justified version, are full rewrites. Comparing t
@@ -278,6 +278,33 @@ void WhitespaceManager::calculateLineBreakInformation() {
}
}
+// Sets the spaces in front of a Change, and updates the start/end columns of
+// subsequent tokens so that trailing comments and escaped newlines can be
+// aligned properly
+static void
+Se
https://github.com/bdunkin updated
https://github.com/llvm/llvm-project/pull/143781
>From f0a9d10d5306e5f074259c88426725b5adfe6994 Mon Sep 17 00:00:00 2001
From: Ben Dunkin
Date: Fri, 6 Jun 2025 14:23:29 -0700
Subject: [PATCH] Rewrite AlignArrayOfStructures implementation to allow
non-rectangu
@@ -3630,6 +3630,36 @@ static unsigned maxNestingDepth(const AnnotatedLine
&Line) {
return Result;
}
+// Returns the token after the first qualifier of the name, or nullptr if there
+// is no qualifier.
+static FormatToken* skipNameQualifier(const FormatToken *Tok) {
--
bdunkin wrote:
I will test my changes against those issues and see what happens. I would very
much like to keep this feature, and am willing to spend some time to make it
right. The issue of not changing existing tests still seems unresolved though.
What happens if fixing some of those issues
bdunkin wrote:
Which existing bugs need to be fixed? I tried to not change the underlying code
too much, but if a redesign is on the table, does that mean backwards
compatibility is a lesser concern?
https://github.com/llvm/llvm-project/pull/143781
_
bdunkin wrote:
Just a friendly bump on this change :)
https://github.com/llvm/llvm-project/pull/143781
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
bdunkin wrote:
I will gladly add a new option, if that is deemed necessary. I just want to
make sure that's the direction I should go. My experience has been it is very
hard to take back an option once it has been released, so they should be added
deliberately, and with careful consideration.
bdunkin wrote:
> You can't change existing tests, you need an option if you want to pad with
> spaces
It is unclear to me if this should be considered a bug fix, or a new feature.
Surely requiring new options are not required for every bug fix.
https://github.com/llvm/llvm-project/pull/143781
@@ -3651,6 +3674,21 @@ static FormatToken *getFunctionName(const AnnotatedLine
&Line,
continue;
}
+// Skip past template typename declarations that may precede the
+// constructor/destructor name
bdunkin wrote:
Done.
https://github.com/llv
https://github.com/bdunkin updated
https://github.com/llvm/llvm-project/pull/143194
>From 839d068df748189470f2c69eb47714425de9524b Mon Sep 17 00:00:00 2001
From: Ben Dunkin
Date: Fri, 6 Jun 2025 12:29:13 -0700
Subject: [PATCH] Fix identifiers not being marked as constructor/destructor
names if
@@ -3659,9 +3697,23 @@ static FormatToken *getFunctionName(const AnnotatedLine
&Line,
}
// Skip to the unqualified part of the name.
-while (Tok->startsSequence(tok::identifier, tok::coloncolon)) {
- assert(Tok->Next);
- Tok = Tok->Next->Next;
+while
@@ -3622,6 +3622,29 @@ static unsigned maxNestingDepth(const AnnotatedLine
&Line) {
return Result;
}
+static bool startsQualifiedName(const FormatToken *Tok) {
+ // Consider: A::B::B()
+ // Tok --^
+ if (Tok->startsSequence(tok::identifier, tok::coloncolo
@@ -3622,6 +3622,29 @@ static unsigned maxNestingDepth(const AnnotatedLine
&Line) {
return Result;
}
+static bool startsQualifiedName(const FormatToken *Tok) {
bdunkin wrote:
Done.
https://github.com/llvm/llvm-project/pull/143194
_
https://github.com/bdunkin updated
https://github.com/llvm/llvm-project/pull/143194
>From 37c7cde072e25eae0409e162c5080830d182f2dd Mon Sep 17 00:00:00 2001
From: Ben Dunkin
Date: Fri, 6 Jun 2025 12:29:13 -0700
Subject: [PATCH] Fix identifiers not being marked as constructor/destructor
names if
@@ -3651,6 +3674,21 @@ static FormatToken *getFunctionName(const AnnotatedLine
&Line,
continue;
}
+// Skip past template typename declarations that may precede the
+// constructor/destructor name
+if (Tok->is(tok::kw_template)) {
bdunkin
https://github.com/bdunkin edited
https://github.com/llvm/llvm-project/pull/143781
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/bdunkin created
https://github.com/llvm/llvm-project/pull/143781
This change implements `AlignArrayOfStructures` for non-rectangular arrays
(arrays of arrays where the inner arrays do not all have the same number of
elements).
It is largely backwards compatible with existin
https://github.com/bdunkin updated
https://github.com/llvm/llvm-project/pull/143194
>From 0168891771b3cdfc6f6305b046005fb335cbff01 Mon Sep 17 00:00:00 2001
From: Ben Dunkin
Date: Fri, 6 Jun 2025 12:29:13 -0700
Subject: [PATCH] Fix identifiers not being marked as constructor/destructor
names if
https://github.com/bdunkin created
https://github.com/llvm/llvm-project/pull/143194
This fixes the `SpaceBeforeParensOptions.AfterFunctionDeclarationName` and
`SpaceBeforeParensOptions.AfterFunctionDefinitionName` options not adding
spaces when a template type's constructor or destructor is fo
bdunkin wrote:
Ah ok, I understand how your change is a better fix. I will close this PR as
yours also fixes things. I have some more fixes coming for this same option, so
I will follow your lead on getting the token type of the opening parenthesis
correct.
https://github.com/llvm/llvm-projec
https://github.com/bdunkin closed
https://github.com/llvm/llvm-project/pull/143047
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/bdunkin created
https://github.com/llvm/llvm-project/pull/143047
This fixes the `SpaceBeforeParensOptions.AfterFunctionDeclarationName` and
`SpaceBeforeParensOptions.AfterFunctionDefinitionName` options not adding
spaces when the function has an explicit Microsoft calling co
33 matches
Mail list logo
