MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, krasimir.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.
clang-format `AlignConsecutiveMacros` feature causes real problems when using
Win32 resource.h files via the resource editor in Visual Studio when editing RC
files.
VS produces the following for the resource.h files
...
// Microsoft Visual C++ generated include file.
// Used by MyTest.rc
//
#define IDP_OLE_INIT_FAILED 100
#define IDP_FAILED_TO_CREATE 102
#define ID_STATUSBAR_SHOW 108
#define ID_STATUSBAR_TEXT 109
Visual Studio generates a resource.h with Alignment macros which start at 40
characters, but `AlignConsecutiveMacros` will determine the starting point but
assume a minimum of 0 meaning a clang-format will result in:
#define IDP_OLE_INIT_FAILED 100
#define IDP_FAILED_TO_CREATE 102
#define ID_STATUSBAR_SHOW 108
#define ID_STATUSBAR_TEXT 109
This is would be good until you make a new rc file change which results in VS
writing out the resource.h (back in its own form) - (even clang-format VS
plugin to format on save doesn't resolve this. The writing of resource.h seems
outside of the normal save system)
This situation is made worse, if it encounters a long symbol, in the VS case it
treats this as a one off, but clang-format will assume this is the new minimum.
#define IDP_OLE_INIT_FAILED 100
#define IDP_FAILED_TO_CREATE 102
#define ID_VERYVERYVERYVERY_LONG_LONG_LONG_LONG_RESOURCE 33221
#define ID_STATUSBAR_SHOW 108
#define ID_STATUSBAR_TEXT 109
and will become via clang-format
#define IDP_OLE_INIT_FAILED 100
#define IDP_FAILED_TO_CREATE 102
#define ID_VERYVERYVERYVERY_LONG_LONG_LONG_LONG_RESOURCE 33221
#define ID_STATUSBAR_SHOW 108
#define ID_STATUSBAR_TEXT 109
This patch contains 2 new options
1. Set the minimum to a fixed integer (which for VS should be 40), this means
all id numbers will begin at 40
2. An option to allow very long macro names to break the Alignment, meaning the
next line will reset back to the previous configured minimum (40 in our case)
These 2 option have functionality that could be useful on their own, however
the goal is to allow `AlignConsecutiveMacros` to be useful for Windows Win32
developers and minimize the clang-format changes that the RC editor causes.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D112019
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/WhitespaceManager.cpp
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14954,6 +14954,31 @@
Style));
}
+TEST_F(FormatTest, AlignConsecutiveMacrosMinWidth) {
+ FormatStyle Style = getLLVMStyle();
+ Style.AlignConsecutiveMacros = FormatStyle::ACS_Consecutive;
+ verifyFormat("#define a 3\n"
+ "#define bbbb 4\n"
+ "#define ccc (5)",
+ Style);
+ Style.AlignConsecutiveMacrosMinWidth = 30;
+ verifyFormat("#define a 3\n"
+ "#define bbbb 4\n"
+ "#define ccc (5)",
+ Style);
+
+ verifyFormat("#define a 3\n"
+ "#define vvvvvvvvvverrrryyyyylonglonglongmacro 4\n"
+ "#define ccc (5)",
+ Style);
+
+ Style.AlignConsecutiveMacrosIgnoreMax = true;
+ verifyFormat("#define a 3\n"
+ "#define vvvvvvvvvverrrryyyyylonglonglongmacro 4\n"
+ "#define ccc (5)",
+ Style);
+}
+
TEST_F(FormatTest, AlignConsecutiveAssignmentsAcrossEmptyLines) {
FormatStyle Alignment = getLLVMStyle();
Alignment.AlignConsecutiveMacros = FormatStyle::ACS_Consecutive;
@@ -18245,6 +18270,7 @@
TEST_F(FormatTest, ParsesConfigurationBools) {
FormatStyle Style = {};
Style.Language = FormatStyle::LK_Cpp;
+ CHECK_PARSE_BOOL(AlignConsecutiveMacrosIgnoreMax);
CHECK_PARSE_BOOL(AlignTrailingComments);
CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
@@ -18367,6 +18393,9 @@
CHECK_PARSE("AlignConsecutiveAssignments: AcrossEmptyLinesAndComments",
AlignConsecutiveAssignments,
FormatStyle::ACS_AcrossEmptyLinesAndComments);
+ CHECK_PARSE("AlignConsecutiveMacrosMinWidth: 32",
+ AlignConsecutiveMacrosMinWidth, 32u);
+
// For backwards compability, false / true should still parse
CHECK_PARSE("AlignConsecutiveAssignments: false", AlignConsecutiveAssignments,
FormatStyle::ACS_None);
Index: clang/lib/Format/WhitespaceManager.cpp
===================================================================
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -564,11 +564,15 @@
//
// We need to adjust the StartOfTokenColumn of each Change that is on a line
// containing any matching token to be aligned and located after such token.
-static void AlignMacroSequence(
- unsigned &StartOfSequence, unsigned &EndOfSequence, unsigned &MinColumn,
- unsigned &MaxColumn, bool &FoundMatchOnLine,
- std::function<bool(const WhitespaceManager::Change &C)> AlignMacrosMatches,
- SmallVector<WhitespaceManager::Change, 16> &Changes) {
+static void
+AlignMacroSequence(unsigned &StartOfSequence, unsigned &EndOfSequence,
+ unsigned &MinColumn, unsigned &MaxColumn,
+ bool &FoundMatchOnLine,
+ std::function<bool(const WhitespaceManager::Change &C,
+ const FormatStyle &Style)>
+ AlignMacrosMatches,
+ SmallVector<WhitespaceManager::Change, 16> &Changes,
+ const FormatStyle &Style) {
if (StartOfSequence > 0 && StartOfSequence < EndOfSequence) {
FoundMatchOnLine = false;
@@ -583,7 +587,7 @@
// If this is the first matching token to be aligned, remember by how many
// spaces it has to be shifted, so the rest of the changes on the line are
// shifted by the same amount
- if (!FoundMatchOnLine && AlignMacrosMatches(Changes[I])) {
+ if (!FoundMatchOnLine && AlignMacrosMatches(Changes[I], Style)) {
FoundMatchOnLine = true;
Shift = MinColumn - Changes[I].StartOfTokenColumn;
Changes[I].Spaces += Shift;
@@ -596,7 +600,7 @@
}
}
- MinColumn = 0;
+ MinColumn = Style.AlignConsecutiveMacrosMinWidth;
MaxColumn = UINT_MAX;
StartOfSequence = 0;
EndOfSequence = 0;
@@ -606,7 +610,7 @@
if (Style.AlignConsecutiveMacros == FormatStyle::ACS_None)
return;
- auto AlignMacrosMatches = [](const Change &C) {
+ auto AlignMacrosMatches = [](const Change &C, const FormatStyle &Style) {
const FormatToken *Current = C.Tok;
unsigned SpacesRequiredBefore = 1;
@@ -628,6 +632,10 @@
if (!Current->Previous || !Current->Previous->is(tok::pp_define))
return false;
+ if (Style.AlignConsecutiveMacrosIgnoreMax &&
+ C.StartOfTokenColumn > Style.AlignConsecutiveMacrosMinWidth)
+ return false;
+
// For a macro function, 0 spaces are required between the
// identifier and the lparen that opens the parameter list.
// For a simple macro, 1 space is required between the
@@ -635,7 +643,7 @@
return Current->Next->SpacesRequiredBefore == SpacesRequiredBefore;
};
- unsigned MinColumn = 0;
+ unsigned MinColumn = Style.AlignConsecutiveMacrosMinWidth;
unsigned MaxColumn = UINT_MAX;
// Start and end of the token sequence we're processing.
@@ -671,7 +679,8 @@
if (EmptyLineBreak || NoMatchBreak)
AlignMacroSequence(StartOfSequence, EndOfSequence, MinColumn, MaxColumn,
- FoundMatchOnLine, AlignMacrosMatches, Changes);
+ FoundMatchOnLine, AlignMacrosMatches, Changes,
+ Style);
// A new line starts, re-initialize line status tracking bools.
FoundMatchOnLine = false;
@@ -682,7 +691,7 @@
LineIsComment = false;
}
- if (!AlignMacrosMatches(Changes[I]))
+ if (!AlignMacrosMatches(Changes[I], Style))
continue;
FoundMatchOnLine = true;
@@ -702,7 +711,7 @@
EndOfSequence = I;
AlignMacroSequence(StartOfSequence, EndOfSequence, MinColumn, MaxColumn,
- FoundMatchOnLine, AlignMacrosMatches, Changes);
+ FoundMatchOnLine, AlignMacrosMatches, Changes, Style);
}
void WhitespaceManager::alignConsecutiveAssignments() {
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1059,6 +1059,8 @@
LLVMStyle.AlignConsecutiveBitFields = FormatStyle::ACS_None;
LLVMStyle.AlignConsecutiveDeclarations = FormatStyle::ACS_None;
LLVMStyle.AlignConsecutiveMacros = FormatStyle::ACS_None;
+ LLVMStyle.AlignConsecutiveMacrosMinWidth = 0;
+ LLVMStyle.AlignConsecutiveMacrosIgnoreMax = false;
LLVMStyle.AllowAllArgumentsOnNextLine = true;
LLVMStyle.AllowAllParametersOfDeclarationOnNextLine = true;
LLVMStyle.AllowShortEnumsOnASingleLine = true;
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -211,6 +211,39 @@
/// \version 9
AlignConsecutiveStyle AlignConsecutiveMacros;
+ /// Set the Minimum macro alignment width to be used (default 0)
+ /// When used with ``AlignConsecutiveMacros``
+ ///
+ /// Possible values:
+ ///
+ /// (default 0 or not specified)
+ /// \code
+ /// #define IDC_OK 1
+ /// #define IDC_CANCEL 2
+ /// \endcode
+ ///
+ /// When ``AlignConsecutiveMacrosMinWidth`` is set to 40
+ ///
+ /// \code
+ /// #define IDC_OK 1
+ /// #define IDC_CANCEL 2
+ /// \endcode
+ /// \version 14
+ unsigned AlignConsecutiveMacrosMinWidth;
+
+ /// Macros beyond ``AlignConsecutiveMacrosMinWidth`` break the
+ /// ConsecutiveMacroAlignment streak, When used in conjuction
+ /// With ``AlignConsecutiveMacrosMinWidth`` this allows longer macros to
+ /// not cause shorter macros to align.
+ ///
+ /// \code
+ /// #define IDC_OK 1
+ /// #define IDC_VERYVERYVERY_LONG 3
+ /// #define IDC_CANCEL 2
+ /// \endcode
+ /// \version 14
+ bool AlignConsecutiveMacrosIgnoreMax;
+
/// Style of aligning consecutive assignments.
///
/// ``Consecutive`` will result in formattings like:
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -543,6 +543,40 @@
/* some comment */
#define bar(y, z) (y + z)
+**AlignConsecutiveMacrosIgnoreMax** (``Boolean``) :versionbadge:`clang-format 14`
+ Macros beyond ``AlignConsecutiveMacrosMinWidth`` break the
+ ConsecutiveMacroAlignment streak, When used in conjuction
+ With ``AlignConsecutiveMacrosMinWidth`` this allows longer macros to
+ not cause shorter macros to align.
+
+
+ .. code-block:: c++
+
+ #define IDC_OK 1
+ #define IDC_VERYVERYVERY_LONG 3
+ #define IDC_CANCEL 2
+
+**AlignConsecutiveMacrosMinWidth** (``Unsigned``) :versionbadge:`clang-format 14`
+ Set the Minimum macro alignment width to be used (default 0)
+ When used with ``AlignConsecutiveMacros``
+
+ Possible values:
+
+ (default 0 or not specified)
+
+ .. code-block:: c++
+
+ #define IDC_OK 1
+ #define IDC_CANCEL 2
+
+ When ``AlignConsecutiveMacrosMinWidth`` is set to 40
+
+
+ .. code-block:: c++
+
+ #define IDC_OK 1
+ #define IDC_CANCEL 2
+
**AlignEscapedNewlines** (``EscapedNewlineAlignmentStyle``) :versionbadge:`clang-format 5`
Options for aligning backslashes in escaped newlines.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
