[llvm] [llvm-remarkutil] Make invalid states un-representable in the count tool (PR #140829)
Jon Roelofs via llvm-commits
llvm-commits at lists.llvm.org
Tue May 20 18:43:46 PDT 2025
https://github.com/jroelofs created https://github.com/llvm/llvm-project/pull/140829
This consolidates some of the error handling around regex arguments to the tool, and sets up the APIs such that errors must be handled before their usage.
>From 28a34df08f2fe95029012304e547a526fa43d4a8 Mon Sep 17 00:00:00 2001
From: Jon Roelofs <jonathan_roelofs at apple.com>
Date: Mon, 19 May 2025 12:18:12 -0700
Subject: [PATCH] [llvm-remarkutil] Make invalid states un-representable in the
count tool.
This consolidates some of the error handling around regex arguments to the
tool, and sets up the APIs such that errors must be handled before their usage.
---
llvm/tools/llvm-remarkutil/RemarkCounter.cpp | 71 ++++++++++---------
llvm/tools/llvm-remarkutil/RemarkCounter.h | 56 +--------------
.../tools/llvm-remarkutil/RemarkUtilHelpers.h | 33 +++++++++
3 files changed, 73 insertions(+), 87 deletions(-)
diff --git a/llvm/tools/llvm-remarkutil/RemarkCounter.cpp b/llvm/tools/llvm-remarkutil/RemarkCounter.cpp
index 2d9432e41d9c0..c9806b2ba8a17 100644
--- a/llvm/tools/llvm-remarkutil/RemarkCounter.cpp
+++ b/llvm/tools/llvm-remarkutil/RemarkCounter.cpp
@@ -111,19 +111,6 @@ static unsigned getValForKey(StringRef Key, const Remark &Remark) {
return *RemarkArg->getValAsInt();
}
-Error Filters::regexArgumentsValid() {
- if (RemarkNameFilter && RemarkNameFilter->IsRegex)
- if (auto E = checkRegex(RemarkNameFilter->FilterRE))
- return E;
- if (PassNameFilter && PassNameFilter->IsRegex)
- if (auto E = checkRegex(PassNameFilter->FilterRE))
- return E;
- if (ArgFilter && ArgFilter->IsRegex)
- if (auto E = checkRegex(ArgFilter->FilterRE))
- return E;
- return Error::success();
-}
-
bool Filters::filterRemark(const Remark &Remark) {
if (RemarkNameFilter && !RemarkNameFilter->match(Remark.RemarkName))
return false;
@@ -250,27 +237,42 @@ Error RemarkCounter::print(StringRef OutputFileName) {
Expected<Filters> getRemarkFilter() {
// Create Filter properties.
std::optional<FilterMatcher> RemarkNameFilter;
+ if (!RemarkNameOpt.empty())
+ RemarkNameFilter = FilterMatcher::createExact(RemarkNameOpt);
+ else if (!RemarkNameOptRE.empty()) {
+ auto FM = FilterMatcher::createRE(RemarkNameOptRE, "rremark-name");
+ if (!FM)
+ return FM.takeError();
+ RemarkNameFilter = std::move(*FM);
+ }
+
std::optional<FilterMatcher> PassNameFilter;
+ if (!PassNameOpt.empty())
+ PassNameFilter = FilterMatcher::createExact(PassNameOpt);
+ else if (!PassNameOptRE.empty()) {
+ auto FM = FilterMatcher::createRE(PassNameOptRE, "rpass-name");
+ if (!FM)
+ return FM.takeError();
+ PassNameFilter = std::move(*FM);
+ }
+
std::optional<FilterMatcher> RemarkArgFilter;
+ if (!RemarkFilterArgByOpt.empty())
+ RemarkArgFilter = FilterMatcher::createExact(RemarkFilterArgByOpt);
+ else if (!RemarkArgFilterOptRE.empty()) {
+ auto FM = FilterMatcher::createRE(RemarkArgFilterOptRE, "rfilter-arg-by");
+ if (!FM)
+ return FM.takeError();
+ RemarkArgFilter = std::move(*FM);
+ }
+
std::optional<Type> RemarkType;
- if (!RemarkNameOpt.empty())
- RemarkNameFilter = {RemarkNameOpt, false};
- else if (!RemarkNameOptRE.empty())
- RemarkNameFilter = {RemarkNameOptRE, true};
- if (!PassNameOpt.empty())
- PassNameFilter = {PassNameOpt, false};
- else if (!PassNameOptRE.empty())
- PassNameFilter = {PassNameOptRE, true};
if (RemarkTypeOpt != Type::Failure)
RemarkType = RemarkTypeOpt;
- if (!RemarkFilterArgByOpt.empty())
- RemarkArgFilter = {RemarkFilterArgByOpt, false};
- else if (!RemarkArgFilterOptRE.empty())
- RemarkArgFilter = {RemarkArgFilterOptRE, true};
+
// Create RemarkFilter.
- return Filters::createRemarkFilter(std::move(RemarkNameFilter),
- std::move(PassNameFilter),
- std::move(RemarkArgFilter), RemarkType);
+ return Filters{std::move(RemarkNameFilter), std::move(PassNameFilter),
+ std::move(RemarkArgFilter), RemarkType};
}
Error useCollectRemark(StringRef Buffer, Counter &Counter, Filters &Filter) {
@@ -313,12 +315,17 @@ static Error collectRemarks() {
SmallVector<FilterMatcher, 4> ArgumentsVector;
if (!Keys.empty()) {
for (auto &Key : Keys)
- ArgumentsVector.push_back({Key, false});
+ ArgumentsVector.push_back(FilterMatcher::createExact(Key));
} else if (!RKeys.empty())
- for (auto Key : RKeys)
- ArgumentsVector.push_back({Key, true});
+ for (auto Key : RKeys) {
+ auto FM = FilterMatcher::createRE(Key, "count-by");
+ if (!FM)
+ return FM.takeError();
+ ArgumentsVector.push_back(std::move(*FM));
+ }
else
- ArgumentsVector.push_back({".*", true});
+ ArgumentsVector.push_back(
+ cantFail(FilterMatcher::createRE(".*", "count-by")));
Expected<ArgumentCounter> AC = ArgumentCounter::createArgumentCounter(
GroupByOpt, ArgumentsVector, Buffer, Filter);
diff --git a/llvm/tools/llvm-remarkutil/RemarkCounter.h b/llvm/tools/llvm-remarkutil/RemarkCounter.h
index 34d5bff774055..3b977791d87c2 100644
--- a/llvm/tools/llvm-remarkutil/RemarkCounter.h
+++ b/llvm/tools/llvm-remarkutil/RemarkCounter.h
@@ -45,26 +45,6 @@ inline std::string groupByToStr(GroupBy GroupBy) {
}
}
-/// Filter object which can be either a string or a regex to match with the
-/// remark properties.
-struct FilterMatcher {
- Regex FilterRE;
- std::string FilterStr;
- bool IsRegex;
- FilterMatcher(std::string Filter, bool IsRegex) : IsRegex(IsRegex) {
- if (IsRegex)
- FilterRE = Regex(Filter);
- else
- FilterStr = Filter;
- }
-
- bool match(StringRef StringToMatch) const {
- if (IsRegex)
- return FilterRE.match(StringToMatch);
- return FilterStr == StringToMatch.trim().str();
- }
-};
-
/// Filter out remarks based on remark properties based on name, pass name,
/// argument and type.
struct Filters {
@@ -72,39 +52,11 @@ struct Filters {
std::optional<FilterMatcher> PassNameFilter;
std::optional<FilterMatcher> ArgFilter;
std::optional<Type> RemarkTypeFilter;
- /// Returns a filter object if all the arguments provided are valid regex
- /// types otherwise return an error.
- static Expected<Filters>
- createRemarkFilter(std::optional<FilterMatcher> RemarkNameFilter,
- std::optional<FilterMatcher> PassNameFilter,
- std::optional<FilterMatcher> ArgFilter,
- std::optional<Type> RemarkTypeFilter) {
- Filters Filter;
- Filter.RemarkNameFilter = std::move(RemarkNameFilter);
- Filter.PassNameFilter = std::move(PassNameFilter);
- Filter.ArgFilter = std::move(ArgFilter);
- Filter.RemarkTypeFilter = std::move(RemarkTypeFilter);
- if (auto E = Filter.regexArgumentsValid())
- return std::move(E);
- return std::move(Filter);
- }
+
/// Returns true if \p Remark satisfies all the provided filters.
bool filterRemark(const Remark &Remark);
-
-private:
- /// Check if arguments can be parsed as valid regex types.
- Error regexArgumentsValid();
};
-/// Convert Regex string error to an error object.
-inline Error checkRegex(const Regex &Regex) {
- std::string Error;
- if (!Regex.isValid(Error))
- return createStringError(make_error_code(std::errc::invalid_argument),
- Twine("Regex: ", Error));
- return Error::success();
-}
-
/// Abstract counter class used to define the general required methods for
/// counting a remark.
struct Counter {
@@ -160,12 +112,6 @@ struct ArgumentCounter : Counter {
StringRef Buffer, Filters &Filter) {
ArgumentCounter AC;
AC.Group = Group;
- for (auto &Arg : Arguments) {
- if (Arg.IsRegex) {
- if (auto E = checkRegex(Arg.FilterRE))
- return std::move(E);
- }
- }
if (auto E = AC.getAllMatchingArgumentsInRemark(Buffer, Arguments, Filter))
return std::move(E);
return AC;
diff --git a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
index 5d2335224d4c2..31a64ce668ed1 100644
--- a/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
+++ b/llvm/tools/llvm-remarkutil/RemarkUtilHelpers.h
@@ -18,6 +18,7 @@
#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Regex.h"
#include "llvm/Support/ToolOutputFile.h"
// Keep input + output help + names consistent across the various modes via a
@@ -55,5 +56,37 @@ Expected<std::unique_ptr<ToolOutputFile>>
getOutputFileWithFlags(StringRef OutputFileName, sys::fs::OpenFlags Flags);
Expected<std::unique_ptr<ToolOutputFile>>
getOutputFileForRemarks(StringRef OutputFileName, Format OutputFormat);
+
+/// Filter object which can be either a string or a regex to match with the
+/// remark properties.
+class FilterMatcher {
+ Regex FilterRE;
+ std::string FilterStr;
+ bool IsRegex;
+
+ FilterMatcher(StringRef Filter, bool IsRegex)
+ : FilterRE(Filter), FilterStr(Filter), IsRegex(IsRegex) {}
+
+public:
+ static FilterMatcher createExact(StringRef Filter) { return {Filter, false}; }
+
+ static Expected<FilterMatcher> createRE(StringRef Filter,
+ StringRef Argument) {
+ FilterMatcher FM(Filter, true);
+ std::string Error;
+ if (!FM.FilterRE.isValid(Error))
+ return createStringError(make_error_code(std::errc::invalid_argument),
+ "invalid argument '--" + Argument + "=" +
+ Filter + "': " + Error);
+ return std::move(FM);
+ }
+
+ bool match(StringRef StringToMatch) const {
+ if (IsRegex)
+ return FilterRE.match(StringToMatch);
+ return FilterStr == StringToMatch.trim().str();
+ }
+};
+
} // namespace remarks
} // namespace llvm
More information about the llvm-commits
mailing list