Skip to content

[NFCI] Change SpecialCaseList::inSectionBlame to return pair<uint, uint> (FileIdx, LineNo). #141540

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 28, 2025
Merged

[NFCI] Change SpecialCaseList::inSectionBlame to return pair<uint, uint> (FileIdx, LineNo). #141540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e7540c3
[𝘀𝗽𝗿] changes to main this commit is based on
qinkunbao May 27, 2025
98dd039
[𝘀𝗽𝗿] initial version
qinkunbao May 27, 2025
d5508cc
Format SpecialCaseList.h
qinkunbao May 27, 2025
b094fc2
Remove irelevant format changes
qinkunbao May 27, 2025
561e98b
review
qinkunbao May 28, 2025
a898b89
NotFound
qinkunbao May 28, 2025
cba26f2
static constexpr
qinkunbao May 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions llvm/include/llvm/Support/SpecialCaseList.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "llvm/Support/Regex.h"
#include <memory>
#include <string>
#include <utility>
#include <vector>

namespace llvm {
Expand Down Expand Up @@ -69,6 +70,7 @@ class FileSystem;
/// ---
class SpecialCaseList {
public:
static constexpr std::pair<unsigned, unsigned> NotFound = {0, 0};
Copy link
Member Author

@qinkunbao qinkunbao May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought static constexpr and constexpr static mean exactly the same thing

Copy link
Collaborator

@vitalybuka vitalybuka May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, for consistency

/// Parses the special case list entries from files. On failure, returns
/// 0 and writes an error message to string.
LLVM_ABI static std::unique_ptr<SpecialCaseList>
Expand All @@ -93,17 +95,17 @@ class SpecialCaseList {
LLVM_ABI bool inSection(StringRef Section, StringRef Prefix, StringRef Query,
StringRef Category = StringRef()) const;

/// Returns the line number corresponding to the special case list entry if
/// the special case list contains a line
/// Returns the file index and the line number <FileIdx, LineNo> corresponding
/// to the special case list entry if the special case list contains a line
/// \code
/// @Prefix:<E>=@Category
/// \endcode
/// where @Query satisfies the glob <E> in a given @Section.
/// Returns zero if there is no exclusion entry corresponding to this
/// Returns (zero, zero) if there is no exclusion entry corresponding to this
/// expression.
LLVM_ABI unsigned inSectionBlame(StringRef Section, StringRef Prefix,
StringRef Query,
StringRef Category = StringRef()) const;
LLVM_ABI std::pair<unsigned, unsigned>
inSectionBlame(StringRef Section, StringRef Prefix, StringRef Query,
StringRef Category = StringRef()) const;

protected:
// Implementations of the create*() functions that can also be used by derived
Expand Down Expand Up @@ -142,21 +144,24 @@ class SpecialCaseList {
using SectionEntries = StringMap<StringMap<Matcher>>;

struct Section {
Section(std::unique_ptr<Matcher> M) : SectionMatcher(std::move(M)) {};
Section() : Section(std::make_unique<Matcher>()) {};
Section(StringRef Str, unsigned FileIdx)
: SectionStr(Str), FileIdx(FileIdx) {};

std::unique_ptr<Matcher> SectionMatcher;
std::unique_ptr<Matcher> SectionMatcher = std::make_unique<Matcher>();
SectionEntries Entries;
std::string SectionStr;
unsigned FileIdx;
};

std::vector<Section> Sections;

LLVM_ABI Expected<Section *> addSection(StringRef SectionStr, unsigned LineNo,
LLVM_ABI Expected<Section *> addSection(StringRef SectionStr,
unsigned FileIdx, unsigned LineNo,
bool UseGlobs = true);

/// Parses just-constructed SpecialCaseList entries from a memory buffer.
LLVM_ABI bool parse(const MemoryBuffer *MB, std::string &Error);
LLVM_ABI bool parse(unsigned FileIdx, const MemoryBuffer *MB,
std::string &Error);

// Helper method for derived classes to search by Prefix, Query, and Category
// once they have already resolved a section entry.
Expand Down
35 changes: 19 additions & 16 deletions llvm/lib/Support/SpecialCaseList.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,16 @@ SpecialCaseList::createOrDie(const std::vector<std::string> &Paths,

bool SpecialCaseList::createInternal(const std::vector<std::string> &Paths,
vfs::FileSystem &VFS, std::string &Error) {
for (const auto &Path : Paths) {
for (size_t i = 0; i < Paths.size(); ++i) {
const auto &Path = Paths[i];
ErrorOr<std::unique_ptr<MemoryBuffer>> FileOrErr =
VFS.getBufferForFile(Path);
if (std::error_code EC = FileOrErr.getError()) {
Error = (Twine("can't open file '") + Path + "': " + EC.message()).str();
return false;
}
std::string ParseError;
if (!parse(FileOrErr.get().get(), ParseError)) {
if (!parse(i, FileOrErr.get().get(), ParseError)) {
Error = (Twine("error parsing file '") + Path + "': " + ParseError).str();
return false;
}
Expand All @@ -123,17 +124,16 @@ bool SpecialCaseList::createInternal(const std::vector<std::string> &Paths,

bool SpecialCaseList::createInternal(const MemoryBuffer *MB,
std::string &Error) {
if (!parse(MB, Error))
if (!parse(0, MB, Error))
return false;
return true;
}

Expected<SpecialCaseList::Section *>
SpecialCaseList::addSection(StringRef SectionStr, unsigned LineNo,
bool UseGlobs) {
Sections.emplace_back();
SpecialCaseList::addSection(StringRef SectionStr, unsigned FileNo,
unsigned LineNo, bool UseGlobs) {
Sections.emplace_back(SectionStr, FileNo);
auto &Section = Sections.back();
Section.SectionStr = SectionStr;

if (auto Err = Section.SectionMatcher->insert(SectionStr, LineNo, UseGlobs)) {
return createStringError(errc::invalid_argument,
Expand All @@ -145,9 +145,10 @@ SpecialCaseList::addSection(StringRef SectionStr, unsigned LineNo,
return &Section;
}

bool SpecialCaseList::parse(const MemoryBuffer *MB, std::string &Error) {
bool SpecialCaseList::parse(unsigned FileIdx, const MemoryBuffer *MB,
std::string &Error) {
Section *CurrentSection;
if (auto Err = addSection("*", 1).moveInto(CurrentSection)) {
if (auto Err = addSection("*", FileIdx, 1).moveInto(CurrentSection)) {
Error = toString(std::move(Err));
return false;
}
Expand Down Expand Up @@ -175,7 +176,8 @@ bool SpecialCaseList::parse(const MemoryBuffer *MB, std::string &Error) {
return false;
}

if (auto Err = addSection(Line.drop_front().drop_back(), LineNo, UseGlobs)
if (auto Err = addSection(Line.drop_front().drop_back(), FileIdx, LineNo,
UseGlobs)
.moveInto(CurrentSection)) {
Error = toString(std::move(Err));
return false;
Expand Down Expand Up @@ -208,20 +210,21 @@ SpecialCaseList::~SpecialCaseList() = default;

bool SpecialCaseList::inSection(StringRef Section, StringRef Prefix,
StringRef Query, StringRef Category) const {
return inSectionBlame(Section, Prefix, Query, Category);
auto [FileIdx, LineNo] = inSectionBlame(Section, Prefix, Query, Category);
return LineNo;
}

unsigned SpecialCaseList::inSectionBlame(StringRef Section, StringRef Prefix,
StringRef Query,
StringRef Category) const {
std::pair<unsigned, unsigned>
SpecialCaseList::inSectionBlame(StringRef Section, StringRef Prefix,
StringRef Query, StringRef Category) const {
for (const auto &S : reverse(Sections)) {
if (S.SectionMatcher->match(Section)) {
unsigned Blame = inSectionBlame(S.Entries, Prefix, Query, Category);
if (Blame)
return Blame;
return {S.FileIdx, Blame};
}
}
return 0;
return NotFound;
}

unsigned SpecialCaseList::inSectionBlame(const SectionEntries &Entries,
Expand Down
15 changes: 9 additions & 6 deletions llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ static void printBlameContext(const DILineInfo &LineInfo, unsigned Context) {
File->getBuffer().split(Lines, '\n');

for (unsigned i = std::max<size_t>(1, LineInfo.Line - Context);
i <
std::min<size_t>(Lines.size() + 1, LineInfo.Line + Context + 1);
i < std::min<size_t>(Lines.size() + 1, LineInfo.Line + Context + 1);
++i) {
if (i == LineInfo.Line)
outs() << ">";
Expand Down Expand Up @@ -193,12 +192,16 @@ printIndirectCFInstructions(FileAnalysis &Analysis,

unsigned BlameLine = 0;
for (auto &K : {"cfi-icall", "cfi-vcall"}) {
if (!BlameLine)
BlameLine =
if (!BlameLine) {
auto [FileIdx, Line] =
SpecialCaseList->inSectionBlame(K, "src", LineInfo.FileName);
if (!BlameLine)
BlameLine =
BlameLine = Line;
}
if (!BlameLine) {
auto [FileIdx, Line] =
SpecialCaseList->inSectionBlame(K, "fun", LineInfo.FunctionName);
BlameLine = Line;
}
}

if (BlameLine) {
Expand Down
57 changes: 41 additions & 16 deletions llvm/unittests/Support/SpecialCaseListTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "gtest/gtest.h"

using testing::HasSubstr;
using testing::Pair;
using testing::StartsWith;
using namespace llvm;

Expand Down Expand Up @@ -70,13 +71,14 @@ TEST_F(SpecialCaseListTest, Basic) {
EXPECT_FALSE(SCL->inSection("", "fun", "hello"));
EXPECT_FALSE(SCL->inSection("", "src", "hello", "category"));

EXPECT_EQ(3u, SCL->inSectionBlame("", "src", "hello"));
EXPECT_EQ(4u, SCL->inSectionBlame("", "src", "bye"));
EXPECT_EQ(5u, SCL->inSectionBlame("", "src", "hi", "category"));
EXPECT_EQ(6u, SCL->inSectionBlame("", "src", "zzzz", "category"));
EXPECT_EQ(0u, SCL->inSectionBlame("", "src", "hi"));
EXPECT_EQ(0u, SCL->inSectionBlame("", "fun", "hello"));
EXPECT_EQ(0u, SCL->inSectionBlame("", "src", "hello", "category"));
EXPECT_THAT(SCL->inSectionBlame("", "src", "hello"), Pair(0u, 3u));
EXPECT_THAT(SCL->inSectionBlame("", "src", "bye"), Pair(0u, 4u));
EXPECT_THAT(SCL->inSectionBlame("", "src", "hi", "category"), Pair(0u, 5u));
EXPECT_THAT(SCL->inSectionBlame("", "src", "zzzz", "category"), Pair(0u, 6u));
EXPECT_THAT(SCL->inSectionBlame("", "src", "hi"), Pair(0u, 0u));
EXPECT_THAT(SCL->inSectionBlame("", "fun", "hello"), Pair(0u, 0u));
EXPECT_THAT(SCL->inSectionBlame("", "src", "hello", "category"),
Pair(0u, 0u));
}

TEST_F(SpecialCaseListTest, CorrectErrorLineNumberWithBlankLine) {
Expand Down Expand Up @@ -311,8 +313,8 @@ TEST_F(SpecialCaseListTest, LinesInSection) {
std::unique_ptr<SpecialCaseList> SCL = makeSpecialCaseList("fun:foo\n"
"fun:bar\n"
"fun:foo\n");
EXPECT_EQ(3u, SCL->inSectionBlame("sect1", "fun", "foo"));
EXPECT_EQ(2u, SCL->inSectionBlame("sect1", "fun", "bar"));
EXPECT_THAT(SCL->inSectionBlame("sect1", "fun", "foo"), Pair(0u, 3u));
EXPECT_THAT(SCL->inSectionBlame("sect1", "fun", "bar"), Pair(0u, 2u));
}

TEST_F(SpecialCaseListTest, LinesCrossSection) {
Expand All @@ -321,8 +323,8 @@ TEST_F(SpecialCaseListTest, LinesCrossSection) {
"fun:foo\n"
"[sect1]\n"
"fun:bar\n");
EXPECT_EQ(3u, SCL->inSectionBlame("sect1", "fun", "foo"));
EXPECT_EQ(5u, SCL->inSectionBlame("sect1", "fun", "bar"));
EXPECT_THAT(SCL->inSectionBlame("sect1", "fun", "foo"), Pair(0u, 3u));
EXPECT_THAT(SCL->inSectionBlame("sect1", "fun", "bar"), Pair(0u, 5u));
}

TEST_F(SpecialCaseListTest, Blame) {
Expand All @@ -341,10 +343,33 @@ TEST_F(SpecialCaseListTest, Blame) {
EXPECT_TRUE(SCL->inSection("sect2", "src", "def"));
EXPECT_TRUE(SCL->inSection("sect1", "src", "def"));

EXPECT_EQ(2u, SCL->inSectionBlame("sect1", "src", "fooz"));
EXPECT_EQ(4u, SCL->inSectionBlame("sect1", "src", "barz"));
EXPECT_EQ(5u, SCL->inSectionBlame("sect1", "src", "def"));
EXPECT_EQ(8u, SCL->inSectionBlame("sect2", "src", "def"));
EXPECT_EQ(8u, SCL->inSectionBlame("sect2", "src", "dez"));
EXPECT_THAT(SCL->inSectionBlame("sect1", "src", "fooz"), Pair(0u, 2u));
EXPECT_THAT(SCL->inSectionBlame("sect1", "src", "barz"), Pair(0u, 4u));
EXPECT_THAT(SCL->inSectionBlame("sect1", "src", "def"), Pair(0u, 5u));
EXPECT_THAT(SCL->inSectionBlame("sect2", "src", "def"), Pair(0u, 8u));
EXPECT_THAT(SCL->inSectionBlame("sect2", "src", "dez"), Pair(0u, 8u));
}

TEST_F(SpecialCaseListTest, FileIdx) {
std::vector<std::string> Files;
Files.push_back(makeSpecialCaseListFile("src:bar\n"
"src:*foo*\n"
"src:ban=init\n"
"src:baz\n"
"src:*def\n"));
Files.push_back(makeSpecialCaseListFile("src:baz\n"
"src:car\n"
"src:def*"));
auto SCL = SpecialCaseList::createOrDie(Files, *vfs::getRealFileSystem());
EXPECT_THAT(SCL->inSectionBlame("", "src", "bar"), Pair(0u, 1u));
EXPECT_THAT(SCL->inSectionBlame("", "src", "fooaaaaaa"), Pair(0u, 2u));
EXPECT_THAT(SCL->inSectionBlame("", "src", "ban", "init"), Pair(0u, 3u));
EXPECT_THAT(SCL->inSectionBlame("", "src", "baz"), Pair(1u, 1u));
EXPECT_THAT(SCL->inSectionBlame("", "src", "car"), Pair(1u, 2u));
EXPECT_THAT(SCL->inSectionBlame("", "src", "aaaadef"), Pair(0u, 5u));
EXPECT_THAT(SCL->inSectionBlame("", "src", "defaaaaa"), Pair(1u, 3u));
for (auto &Path : Files)
sys::fs::remove(Path);
}

} // namespace
Loading