llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-static-analyzer-1 Author: Alex (filaka771) <details> <summary>Changes</summary> Fix a formatting bug in `AnalyzerOptions::printFormattedEntry` (used by `clang -cc1 -print-analyzer-options`), which led to misalignment of a checker description. It happened when a checker name was exactly `EntryWidth` — a redundant space was inserted. **Before the fix (misaligned entry is underlined with a red line):** <img width="2205" height="729" alt="image" src="https://github.com/user-attachments/assets/74df3791-37ea-4522-b052-0925136c080c"; /> # Changes This small PR fixes issues with alignment in `AnalyzerOptions::printFormattedEntry`: - Make `printFormattedEntry` correctly handle the corner case, when a checker's name equals exactly `EntryWidth` (as shown in the "Before the fix" image). I replaced `<` with `<=` to insert `'\n'` in the corner case. - Make `printFormattedEntry` correctly handle corner case, when pad before checker's name (specified by `InitialPad`) equals 0. Before the fix, due to `llvm::raw_formatted_ostream::PadToColumn` logic, when `InitialPad = 0`, it still pads to 1. Since `InitialPad = 0` is never used in the program, this bug is not visible to the user. I added a test that explicitly checks for this. - I believe that the easiest way to check for these behaviours is by leveraging the unittest system. I could instead write the usual test to check for exact formatting of `-print-analyzer-options`, but I thought that it would be too fragile. **After the fix (now underlined entry aligned correctly):** <img width="2187" height="757" alt="image" src="https://github.com/user-attachments/assets/6896c386-8525-4bc8-a032-9833699cfccf"; /> What do you think about the changes? I am happy to answer any questions or make adjustments. --- Full diff: https://github.com/llvm/llvm-project/pull/190570.diff 3 Files Affected: - (modified) clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp (+8-3) - (added) clang/unittests/StaticAnalyzer/AnalyzerFormattingTest.cpp (+180) - (modified) clang/unittests/StaticAnalyzer/CMakeLists.txt (+1) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp index 6ba67f8d11d57..20bcbdb4762d0 100644 --- a/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp +++ b/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp @@ -37,9 +37,14 @@ void AnalyzerOptions::printFormattedEntry( const size_t PadForDesc = InitialPad + EntryWidth; - FOut.PadToColumn(InitialPad) << EntryDescPair.first; - // If the buffer's length is greater than PadForDesc, print a newline. - if (FOut.getColumn() > PadForDesc) + if (InitialPad != 0) + FOut.PadToColumn(InitialPad); + + FOut << EntryDescPair.first; + + // If the buffer's length is greater than or equal to PadForDesc, + // print a newline. + if (FOut.getColumn() >= PadForDesc) FOut << '\n'; FOut.PadToColumn(PadForDesc); diff --git a/clang/unittests/StaticAnalyzer/AnalyzerFormattingTest.cpp b/clang/unittests/StaticAnalyzer/AnalyzerFormattingTest.cpp new file mode 100644 index 0000000000000..707e3197d0eab --- /dev/null +++ b/clang/unittests/StaticAnalyzer/AnalyzerFormattingTest.cpp @@ -0,0 +1,180 @@ +//===- AnalyzerFormattingTest.cpp - SA Formatting test --------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file contains tests for printFormattedEntry function, which is used for +// printing available analyzers and their descriptions. +// +//===----------------------------------------------------------------------===// + +#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/Support/raw_ostream.h" +#include "gtest/gtest.h" + +using namespace clang::ento; + +static void runPrintFormattedEntry( + std::pair<llvm::StringRef, llvm::StringRef> EntryDescPair, + size_t InitialPad, size_t EntryWidth, size_t MinLineWidth, + llvm::SmallString<256> &OutBuffer) { + llvm::raw_svector_ostream Out(OutBuffer); + clang::AnalyzerOptions::printFormattedEntry(Out, EntryDescPair, InitialPad, + EntryWidth, MinLineWidth); +} + +// No wrapping after checker's name. +// No initial pad. +TEST(PrintFormattedEntryTest, SimplePrint) { + llvm::SmallString<256> Buffer; + runPrintFormattedEntry(/*EntryDescPair=*/{"Checkers", "A description"}, + /*InitialPad=*/0, + /*EntryWidth=*/20, + /*MinLineWidth=*/0, + /*OutBuffer=*/Buffer); + + EXPECT_EQ(Buffer, "Checkers A description"); +} + +// With wrapping after checker's name. +// No initial pad. +TEST(PrintFormattedEntryTest, EntryLongerThanWidth) { + llvm::SmallString<256> Buffer; + runPrintFormattedEntry( + {/*EntryDescPair=*/"VeryLongCheckerName", "A description"}, + /*InitialPad=*/0, + /*EntryWidth=*/10, + /*MinLineWidth=*/0, + /*OutBuffer=*/Buffer); + + EXPECT_EQ(Buffer, "VeryLongCheckerName\n" + " A description"); +} + +// With wrapping after checker's name. +// No initial pad. +// Corner case, when checker's name length equal to EntryWidth. +TEST(PrintFormattedEntryTest, ExactFillWidth) { + llvm::SmallString<256> Buffer; + runPrintFormattedEntry(/*EntryDescPair=*/{"Checkers", "A description"}, + /*InitialPad=*/0, + /*EntryWidth=*/8, + /*MinLineWidth=*/0, + /*OutBuffer=*/Buffer); + + EXPECT_EQ(Buffer, "Checkers\n" + " A description"); +} + +// No wrapping after checker's name. +// With initial pad. +TEST(PrintFormattedEntryTest, WithInitialPadding) { + llvm::SmallString<256> Buffer; + runPrintFormattedEntry(/*EntryDescPair=*/{"Checkers", "A description"}, + /*InitialPad=*/2, + /*EntryWidth=*/20, + /*MinLineWidth=*/0, + /*OutBuffer=*/Buffer); + + EXPECT_EQ(Buffer, " Checkers A description"); +} + +// No wrapping after checker's name. +// With initial pad. +// With wrapping in checker's description (MinLineWidth > 0). +TEST(PrintFormattedEntryTest, WrapDescription) { + llvm::SmallString<256> Buffer; + llvm::StringRef Desc = + "This is a long description that should be wrapped into multiple lines."; + runPrintFormattedEntry(/*EntryDescPair=*/{"Checkers", Desc}, + /*InitialPad=*/2, + /*EntryWidth=*/20, + /*MinLineWidth=*/40, + /*OutBuffer=*/Buffer); + + llvm::StringRef Expected = + " Checkers This is a long description\n" + " that should be wrapped\n" + " into multiple lines."; + EXPECT_EQ(Buffer, Expected); +} + +// No wrapping after checker's name. +// With initial pad. +// No wrapping in checker's descriptions (MinLineWdth = 0). +TEST(PrintFormattedEntryTest, NoWrap) { + llvm::SmallString<256> Buffer; + llvm::StringRef Desc = "This is a very long description that will not be " + "wrapped because MinLineWidth=0."; + runPrintFormattedEntry(/*EntryDescPair=*/{"Checkers", Desc}, + /*InitialPad=*/2, + /*EntryWidth=*/20, + /*MinLineWidth=*/0, + /*OutBuffer=*/Buffer); + + std::string Expected = " Checkers " + Desc.str(); + EXPECT_EQ(Buffer, Expected); +} + +// No wrapping after checker's name. +// No initial pad. +// Corner case with empty descriptions. +TEST(PrintFormattedEntryTest, EmptyDescription) { + llvm::SmallString<256> Buffer; + runPrintFormattedEntry(/*EntryDescPair=*/{"Checkers", ""}, + /*InitialPad=*/0, + /*EntryWidth=*/20, + /*MinLineWidth=*/0, + /*OutBuffer=*/Buffer); + + EXPECT_EQ(Buffer, "Checkers "); +} + +// No wrapping after checker's name. +// No initial pad. +// Corner case with empty checker's name. +TEST(PrintFormattedEntryTest, EmptyEntry) { + llvm::SmallString<256> Buffer; + runPrintFormattedEntry(/*EntryDescPair=*/{"", "Some description"}, + /*InitialPad=*/0, + /*EntryWidth=*/20, + /*MinLineWidth=*/0, + /*OutBuffer=*/Buffer); + + EXPECT_EQ(Buffer, " Some description"); +} + +// No wrapping after checker's name. +// With initial pad. +// Narrow MinLineWidth with a word without spaces (no break). +TEST(PrintFormattedEntryTest, NarrowMinLineWidthNoSpaces) { + llvm::SmallString<256> Buffer; + runPrintFormattedEntry( + /*EntryDescPair=*/{"Checkers", "This_is_a_long_word_without_spaces"}, + /*InitialPad=*/2, + /*EntryWidth=*/20, + /*MinLineWidth=*/10, + /*OutBuffer=*/Buffer); + + EXPECT_EQ(Buffer, " Checkers This_is_a_long_word_without_spaces"); +} + +// No wrapping after checker's name. +// With initial pad. +// With wrapping in checker's description. +TEST(PrintFormattedEntryTest, MinLineWidthLessThanPad) { + llvm::SmallString<256> Buffer; + runPrintFormattedEntry(/*EntryDescPair=*/{"Checkers", "short phrase"}, + /*InitialPad=*/2, + /*EntryWidth=*/20, + /*MinLineWidth=*/15, + /*OutBuffer=*/Buffer); + + llvm::StringRef Expected = " Checkers short\n" + " phrase"; + EXPECT_EQ(Buffer, Expected); +} diff --git a/clang/unittests/StaticAnalyzer/CMakeLists.txt b/clang/unittests/StaticAnalyzer/CMakeLists.txt index caf686e2a92e2..ed16a1372bea2 100644 --- a/clang/unittests/StaticAnalyzer/CMakeLists.txt +++ b/clang/unittests/StaticAnalyzer/CMakeLists.txt @@ -1,4 +1,5 @@ add_clang_unittest(StaticAnalysisTests + AnalyzerFormattingTest.cpp AnalyzerOptionsTest.cpp APSIntTypeTest.cpp BlockEntranceCallbackTest.cpp `````````` </details> https://github.com/llvm/llvm-project/pull/190570 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
