Skip to content

[FileCheck] Remove unneeded unique_ptr. NFC. #123216

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 1 commit into from
Jan 16, 2025
Merged

[FileCheck] Remove unneeded unique_ptr. NFC. #123216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 1 addition & 2 deletions llvm/include/llvm/FileCheck/FileCheck.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,7 @@ struct FileCheckString;
class FileCheck {
FileCheckRequest Req;
std::unique_ptr<FileCheckPatternContext> PatternContext;
// C++17 TODO: make this a plain std::vector.
std::unique_ptr<std::vector<FileCheckString>> CheckStrings;
std::vector<FileCheckString> CheckStrings;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't even understand why this was required pre-C++17. Everything seems to work when I remove it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I cannot remember but I have a vague memory that it's related to the std::swap. I see that std::swap has noexcept since C++17, maybe that's the reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting. But that code should be using std::move instead of std::swap. I can feel another patch coming on...

Copy link
Collaborator

Choose a reason for hiding this comment

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

IIRC, prior to C++17, possibly even prior C++14, with at least some compilers (I ran into this with GCC, I believe, but not MSVC), you couldn't have vectors of incomplete types, so that would be why.


public:
explicit FileCheck(FileCheckRequest Req);
Expand Down
19 changes: 9 additions & 10 deletions llvm/lib/FileCheck/FileCheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1766,8 +1766,7 @@ void FileCheckPatternContext::createLineVariable() {
}

FileCheck::FileCheck(FileCheckRequest Req)
: Req(Req), PatternContext(std::make_unique<FileCheckPatternContext>()),
CheckStrings(std::make_unique<std::vector<FileCheckString>>()) {}
: Req(Req), PatternContext(std::make_unique<FileCheckPatternContext>()) {}

FileCheck::~FileCheck() = default;

Expand Down Expand Up @@ -1916,7 +1915,7 @@ bool FileCheck::readCheckFile(
// Verify that CHECK-NEXT/SAME/EMPTY lines have at least one CHECK line before them.
if ((CheckTy == Check::CheckNext || CheckTy == Check::CheckSame ||
CheckTy == Check::CheckEmpty) &&
CheckStrings->empty()) {
CheckStrings.empty()) {
StringRef Type = CheckTy == Check::CheckNext
? "NEXT"
: CheckTy == Check::CheckEmpty ? "EMPTY" : "SAME";
Expand All @@ -1934,8 +1933,8 @@ bool FileCheck::readCheckFile(
}

// Okay, add the string we captured to the output vector and move on.
CheckStrings->emplace_back(P, UsedPrefix, PatternLoc);
std::swap(DagNotMatches, CheckStrings->back().DagNotStrings);
CheckStrings.emplace_back(P, UsedPrefix, PatternLoc);
std::swap(DagNotMatches, CheckStrings.back().DagNotStrings);
DagNotMatches = ImplicitNegativeChecks;
}

Expand All @@ -1962,10 +1961,10 @@ bool FileCheck::readCheckFile(
// Add an EOF pattern for any trailing --implicit-check-not/CHECK-DAG/-NOTs,
// and use the first prefix as a filler for the error message.
if (!DagNotMatches.empty()) {
CheckStrings->emplace_back(
CheckStrings.emplace_back(
Pattern(Check::CheckEOF, PatternContext.get(), LineNumber + 1),
*Req.CheckPrefixes.begin(), SMLoc::getFromPointer(Buffer.data()));
std::swap(DagNotMatches, CheckStrings->back().DagNotStrings);
std::swap(DagNotMatches, CheckStrings.back().DagNotStrings);
}

return false;
Expand Down Expand Up @@ -2676,13 +2675,13 @@ bool FileCheck::checkInput(SourceMgr &SM, StringRef Buffer,
std::vector<FileCheckDiag> *Diags) {
bool ChecksFailed = false;

unsigned i = 0, j = 0, e = CheckStrings->size();
unsigned i = 0, j = 0, e = CheckStrings.size();
while (true) {
StringRef CheckRegion;
if (j == e) {
CheckRegion = Buffer;
} else {
const FileCheckString &CheckLabelStr = (*CheckStrings)[j];
const FileCheckString &CheckLabelStr = CheckStrings[j];
if (CheckLabelStr.Pat.getCheckTy() != Check::CheckLabel) {
++j;
continue;
Expand All @@ -2708,7 +2707,7 @@ bool FileCheck::checkInput(SourceMgr &SM, StringRef Buffer,
PatternContext->clearLocalVars();

for (; i != j; ++i) {
const FileCheckString &CheckStr = (*CheckStrings)[i];
const FileCheckString &CheckStr = CheckStrings[i];

// Check each string within the scanned region, including a second check
// of any final CHECK-LABEL (to verify CHECK-NOT and CHECK-DAG)
Expand Down
Loading