diff --git a/backends/tc/tcenv.py b/backends/tc/tcenv.py index bc2df431618..d7e1c41a7d1 100644 --- a/backends/tc/tcenv.py +++ b/backends/tc/tcenv.py @@ -23,6 +23,7 @@ import time from pathlib import Path from typing import Any, List, Optional +from urllib.parse import unquote # Append tools to the import path. FILE_DIR = Path(__file__).resolve().parent @@ -133,19 +134,29 @@ def __del__(self): testutils.kill_proc_group(self.vm_proc) def _get_version(self, url): - pattern = r"(\d+\.\d+\.0p\d+tc-v\d+-rc\d+)" - - match = re.search(pattern, url) + candidates = [ + # Legacy linux-p4tc kernel release naming. + (r"(\d+\.\d+\.0p\d+tc-v\d+-rc\d+\+?)", True), + # Newer release labels can be plain semantic versions. + (r"[Vv]ersion\s+(\d+\.\d+\.\d+(?:\.\d+)?\+?)", False), + # Fallback for asset names: linux-image-_.deb + (r"linux-image-([^/_]+?)(?:_[^/]+)?(?:\.deb)?$", False), + ] + source = unquote(url.strip()) - if match: + for pattern, force_plus in candidates: + match = re.search(pattern, source) + if not match: + continue version = match.group(1) + if force_plus and not version.endswith("+"): + version += "+" if self.verbose: print(f"Extracted version: {version}") - else: - testutils.log.error("Version string not found") - return None + return version - return version + "+" + testutils.log.error("Version string not found in '%s'", source) + return None def extract_version(self): cmd = f'curl -s {self.release_url} | jq .assets[2].browser_download_url | tr -d \\"' diff --git a/backends/tofino/bf-p4c/mau/mau_power.cpp b/backends/tofino/bf-p4c/mau/mau_power.cpp index 62e8e75cc4b..823e52a6110 100644 --- a/backends/tofino/bf-p4c/mau/mau_power.cpp +++ b/backends/tofino/bf-p4c/mau/mau_power.cpp @@ -22,6 +22,8 @@ #include #include +#include + #include "backends/tofino/bf-p4c/mau/memories.h" #include "backends/tofino/bf-p4c/specs/device.h" #include "lib/hex.h" diff --git a/backends/tofino/bf-p4c/mau/walk_power_graph.cpp b/backends/tofino/bf-p4c/mau/walk_power_graph.cpp index b5f4a17ada8..d792e5caa60 100644 --- a/backends/tofino/bf-p4c/mau/walk_power_graph.cpp +++ b/backends/tofino/bf-p4c/mau/walk_power_graph.cpp @@ -24,6 +24,8 @@ #include #include +#include + #include "backends/tofino/bf-p4c/common/asm_output.h" #include "backends/tofino/bf-p4c/common/run_id.h" #include "backends/tofino/bf-p4c/ir/unique_id.h" diff --git a/backends/tofino/bf-p4c/midend/rewrite_egress_intrinsic_metadata_header.h b/backends/tofino/bf-p4c/midend/rewrite_egress_intrinsic_metadata_header.h index 43dc1709846..a0f74c6fc5b 100644 --- a/backends/tofino/bf-p4c/midend/rewrite_egress_intrinsic_metadata_header.h +++ b/backends/tofino/bf-p4c/midend/rewrite_egress_intrinsic_metadata_header.h @@ -80,8 +80,8 @@ class RewriteEgressIntrinsicMetadataHeader : public P4::PassManager { BUG_CHECK( prev && (canonicalType->width_bits() + prev_type->width_bits()) % 8 == 0, - "%1% not padded to be byte-aligned in %2%", field->name, - "egress_intrinsic_metadata_t"); + "%1% not padded to be byte-aligned in egress_intrinsic_metadata_t", + field->name); new_fields.push_back(prev); } new_fields.push_back(field); diff --git a/backends/tofino/bf-p4c/phv/action_phv_constraints.cpp b/backends/tofino/bf-p4c/phv/action_phv_constraints.cpp index 9408f76094a..5bb054617e5 100644 --- a/backends/tofino/bf-p4c/phv/action_phv_constraints.cpp +++ b/backends/tofino/bf-p4c/phv/action_phv_constraints.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include "backends/tofino/bf-p4c/common/asm_output.h" diff --git a/backends/tofino/bf-p4c/phv/analysis/dominator_tree.cpp b/backends/tofino/bf-p4c/phv/analysis/dominator_tree.cpp index 9c8cbb48991..3cbca076d54 100644 --- a/backends/tofino/bf-p4c/phv/analysis/dominator_tree.cpp +++ b/backends/tofino/bf-p4c/phv/analysis/dominator_tree.cpp @@ -18,6 +18,7 @@ #include "backends/tofino/bf-p4c/phv/analysis/dominator_tree.h" +#include #include #include diff --git a/backends/tofino/bf-p4c/phv/analysis/jbay_phv_analysis.cpp b/backends/tofino/bf-p4c/phv/analysis/jbay_phv_analysis.cpp index 1c0b56aab89..51d766dc837 100644 --- a/backends/tofino/bf-p4c/phv/analysis/jbay_phv_analysis.cpp +++ b/backends/tofino/bf-p4c/phv/analysis/jbay_phv_analysis.cpp @@ -18,9 +18,10 @@ #include "jbay_phv_analysis.h" +#include + #include "backends/tofino/bf-p4c/phv/phv_fields.h" #include "backends/tofino/bf-p4c/phv/phv_parde_mau_use.h" -#include "backends/tofino/bf-p4c/specs/device.h" Visitor::profile_t JbayPhvAnalysis::init_apply(const IR::Node *root) { profile_t rv = Inspector::init_apply(root); diff --git a/backends/tofino/bf-p4c/phv/analysis/meta_live_range.cpp b/backends/tofino/bf-p4c/phv/analysis/meta_live_range.cpp index 0d1ab1a3183..ee0b667f5ef 100644 --- a/backends/tofino/bf-p4c/phv/analysis/meta_live_range.cpp +++ b/backends/tofino/bf-p4c/phv/analysis/meta_live_range.cpp @@ -18,6 +18,8 @@ #include "backends/tofino/bf-p4c/phv/analysis/meta_live_range.h" +#include + cstring MetadataLiveRange::printAccess(int access) { switch (access) { case 1: diff --git a/backends/tofino/bf-p4c/phv/legacy_action_packing_validator.cpp b/backends/tofino/bf-p4c/phv/legacy_action_packing_validator.cpp index 6220387d5c4..3a868183d67 100644 --- a/backends/tofino/bf-p4c/phv/legacy_action_packing_validator.cpp +++ b/backends/tofino/bf-p4c/phv/legacy_action_packing_validator.cpp @@ -21,6 +21,7 @@ #include #include +#include #include #include diff --git a/backends/tofino/bf-p4c/phv/mau_backtracker.cpp b/backends/tofino/bf-p4c/phv/mau_backtracker.cpp index ac18e774a69..2e3975c7855 100644 --- a/backends/tofino/bf-p4c/phv/mau_backtracker.cpp +++ b/backends/tofino/bf-p4c/phv/mau_backtracker.cpp @@ -18,6 +18,8 @@ #include "backends/tofino/bf-p4c/phv/mau_backtracker.h" +#include + #include "backends/tofino/bf-p4c/phv/allocate_phv.h" #include "lib/log.h" diff --git a/backends/tofino/bf-p4c/phv/phv.h b/backends/tofino/bf-p4c/phv/phv.h index d95a8c91652..d44a0bf172a 100644 --- a/backends/tofino/bf-p4c/phv/phv.h +++ b/backends/tofino/bf-p4c/phv/phv.h @@ -19,6 +19,8 @@ #ifndef BACKENDS_TOFINO_BF_P4C_PHV_PHV_H_ #define BACKENDS_TOFINO_BF_P4C_PHV_PHV_H_ +#include + #include "backends/tofino/bf-p4c/specs/phv.h" #include "ir/json_generator.h" diff --git a/backends/tofino/bf-p4c/phv/solver/action_constraint_solver.cpp b/backends/tofino/bf-p4c/phv/solver/action_constraint_solver.cpp index 5d6642ba561..766dcd65038 100644 --- a/backends/tofino/bf-p4c/phv/solver/action_constraint_solver.cpp +++ b/backends/tofino/bf-p4c/phv/solver/action_constraint_solver.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include "backends/tofino/bf-p4c/ir/bitrange.h" diff --git a/backends/tofino/bf-p4c/phv/utils/report.cpp b/backends/tofino/bf-p4c/phv/utils/report.cpp index 95be3e5611c..533e2ec6f89 100644 --- a/backends/tofino/bf-p4c/phv/utils/report.cpp +++ b/backends/tofino/bf-p4c/phv/utils/report.cpp @@ -20,6 +20,8 @@ #include +#include + #include "backends/tofino/bf-p4c/common/table_printer.h" using ContainerAllocStatus = PHV::Allocation::ContainerAllocStatus; diff --git a/backends/tofino/bf-p4c/phv/utils/report.h b/backends/tofino/bf-p4c/phv/utils/report.h index 0adc359fb6b..0122a8d8883 100644 --- a/backends/tofino/bf-p4c/phv/utils/report.h +++ b/backends/tofino/bf-p4c/phv/utils/report.h @@ -19,6 +19,8 @@ #ifndef BACKENDS_TOFINO_BF_P4C_PHV_UTILS_REPORT_H_ #define BACKENDS_TOFINO_BF_P4C_PHV_UTILS_REPORT_H_ +#include + #include "backends/tofino/bf-p4c/phv/utils/utils.h" namespace PHV { diff --git a/backends/tofino/bf-p4c/phv/utils/utils.cpp b/backends/tofino/bf-p4c/phv/utils/utils.cpp index 856902876af..15b4a91add2 100644 --- a/backends/tofino/bf-p4c/phv/utils/utils.cpp +++ b/backends/tofino/bf-p4c/phv/utils/utils.cpp @@ -23,6 +23,7 @@ #include #include +#include #include #include "backends/tofino/bf-p4c/common/table_printer.h" diff --git a/backends/tofino/bf-p4c/phv/v2/action_packing_validator.cpp b/backends/tofino/bf-p4c/phv/v2/action_packing_validator.cpp index 224f9a802e3..8c18f4efae2 100644 --- a/backends/tofino/bf-p4c/phv/v2/action_packing_validator.cpp +++ b/backends/tofino/bf-p4c/phv/v2/action_packing_validator.cpp @@ -21,6 +21,7 @@ #include #include +#include #include #include diff --git a/backends/tofino/bf-p4c/test/gtest/error_reporter.cpp b/backends/tofino/bf-p4c/test/gtest/error_reporter.cpp index c0278a0297b..a198b4caa35 100644 --- a/backends/tofino/bf-p4c/test/gtest/error_reporter.cpp +++ b/backends/tofino/bf-p4c/test/gtest/error_reporter.cpp @@ -76,9 +76,8 @@ class ErrorReporterTest : public ::testing::Test { }; TEST_F(ErrorReporterTest, ErrorHelperPlainFormatsCorrectly) { - boost::format fmt("Str: %1%, Dec: %2%"); - - EXPECT_EQ(::error_helper(fmt, "hello", 10).toString(), "Str: hello, Dec: 10\n"); + EXPECT_EQ(::error_helper("Str: %1%, Dec: %2%", "hello", 10).toString(), + "Str: hello, Dec: 10\n"); } TEST_F(ErrorReporterTest, WarningsConformToExpectedFormat) { diff --git a/frontends/common/constantParsing.cpp b/frontends/common/constantParsing.cpp index 7a8505c351f..78f087f02a1 100644 --- a/frontends/common/constantParsing.cpp +++ b/frontends/common/constantParsing.cpp @@ -95,7 +95,7 @@ int parseConstantChecked(const Util::SourceInfo &srcInfo, const UnparsedConstant auto cst = parseConstant(srcInfo, constant, 0); if (!cst->fitsInt()) { ::P4::error(ErrorType::ERR_OVERLIMIT, - "%1$x: this implementation does not support bitstrings this large", cst); + "%s: this implementation does not support bitstrings this large", cst); return 8; // this is a fine value for a width; compilation will stop anyway } return cst->asInt(); diff --git a/frontends/p4/typeChecking/typeConstraints.cpp b/frontends/p4/typeChecking/typeConstraints.cpp index 62151166244..350ffca7471 100644 --- a/frontends/p4/typeChecking/typeConstraints.cpp +++ b/frontends/p4/typeChecking/typeConstraints.cpp @@ -126,30 +126,30 @@ std::string TypeConstraint::localError(Explain *explainer) const { if (errFormat.isNullOrEmpty()) return ""; std::string message, explanation; - boost::format fmt = boost::format(errFormat.c_str()); switch (errArguments.size()) { case 0: - message = boost::str(fmt); + message = ::P4::createFormattedMessage(errFormat.c_str()); break; case 1: absl::StrAppend(&explanation, explain(0, explainer)); - message = ::P4::error_helper(fmt, errArguments.at(0)).toString(); + message = ::P4::error_helper(errFormat.c_str(), errArguments.at(0)).toString(); break; case 2: absl::StrAppend(&explanation, explain(0, explainer), explain(1, explainer)); - message = ::P4::error_helper(fmt, errArguments.at(0), errArguments.at(1)).toString(); + message = ::P4::error_helper(errFormat.c_str(), errArguments.at(0), errArguments.at(1)) + .toString(); break; case 3: absl::StrAppend(&explanation, explain(0, explainer), explain(1, explainer), explain(2, explainer)); - message = - ::P4::error_helper(fmt, errArguments.at(0), errArguments.at(1), errArguments.at(2)) - .toString(); + message = ::P4::error_helper(errFormat.c_str(), errArguments.at(0), errArguments.at(1), + errArguments.at(2)) + .toString(); break; case 4: absl::StrAppend(&explanation, explain(0, explainer), explain(1, explainer), explain(2, explainer), explain(3, explainer)); - message = ::P4::error_helper(fmt, errArguments.at(0), errArguments.at(1), + message = ::P4::error_helper(errFormat.c_str(), errArguments.at(0), errArguments.at(1), errArguments.at(2), errArguments.at(3)) .toString(); break; diff --git a/frontends/p4/typeChecking/typeConstraints.h b/frontends/p4/typeChecking/typeConstraints.h index 7329f2e4143..9a178a250d1 100644 --- a/frontends/p4/typeChecking/typeConstraints.h +++ b/frontends/p4/typeChecking/typeConstraints.h @@ -92,10 +92,9 @@ class TypeConstraint : public IHasDbPrint, public ICastable { /// composed in reverse order, from bottom to top. The top of the stack /// has no 'derivedFrom' field, and it contains the actual source /// position where the analysis started. - boost::format fmt(format); - return reportErrorImpl(subst, - " ---- Actual error:\n" + - ::P4::error_helper(fmt, std::forward(args)...).toString()); + return reportErrorImpl( + subst, " ---- Actual error:\n" + + ::P4::error_helper(format, std::forward(args)...).toString()); } // Default error message; returns 'false' virtual bool reportError(const TypeVariableSubstitution *subst) const = 0; diff --git a/ir/expression.def b/ir/expression.def index 2a6ab907efb..c92af5bff08 100644 --- a/ir/expression.def +++ b/ir/expression.def @@ -318,25 +318,25 @@ class Constant : Literal { inline bool fitsInt64() const { return value >= INT64_MIN && value <= INT64_MAX; } inline long asLong() const { if (!fitsLong()) - ::P4::error(ErrorType::ERR_OVERLIMIT, "%1$x: Value too large for long", this); + ::P4::error(ErrorType::ERR_OVERLIMIT, "%s: Value too large for long", this); return static_cast(value); } inline int asInt() const { if (!fitsInt()) - ::P4::error(ErrorType::ERR_OVERLIMIT, "%1$x: Value too large for int", this); + ::P4::error(ErrorType::ERR_OVERLIMIT, "%s: Value too large for int", this); return static_cast(value); } inline unsigned asUnsigned() const { if (!fitsUint()) - ::P4::error(ErrorType::ERR_OVERLIMIT, "%1$x: Value too large for unsigned int", this); + ::P4::error(ErrorType::ERR_OVERLIMIT, "%s: Value too large for unsigned int", this); return static_cast(value); } inline uint64_t asUint64() const { if (!fitsUint64()) - ::P4::error(ErrorType::ERR_OVERLIMIT, "%1$x: Value too large for uint64", this); + ::P4::error(ErrorType::ERR_OVERLIMIT, "%s: Value too large for uint64", this); return static_cast(value); } inline int64_t asInt64() const { if (!fitsInt64()) - ::P4::error(ErrorType::ERR_OVERLIMIT, "%1$x: Value too large for int64", this); + ::P4::error(ErrorType::ERR_OVERLIMIT, "%s: Value too large for int64", this); return static_cast(value); } // The following operators are only used in special circumstances. diff --git a/lib/bitrange.h b/lib/bitrange.h index 7604f4c4efe..cd9c7def85c 100644 --- a/lib/bitrange.h +++ b/lib/bitrange.h @@ -25,6 +25,7 @@ limitations under the License. #include "absl/numeric/bits.h" #include "bitvec.h" +#include "cstring.h" #include "exceptions.h" #include "hash.h" diff --git a/lib/boost_format_helper.h b/lib/boost_format_helper.h new file mode 100644 index 00000000000..9fedabf3322 --- /dev/null +++ b/lib/boost_format_helper.h @@ -0,0 +1,349 @@ +#ifndef LIB_BOOST_FORMAT_HELPER_H_ +#define LIB_BOOST_FORMAT_HELPER_H_ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "absl/strings/str_cat.h" +#include "absl/strings/str_format.h" +#include "lib/cstring.h" +#include "lib/source_file.h" +#include "lib/stringify.h" + +namespace P4 { + +namespace detail { +// Formatter adapter for legacy boost::format placeholders (%N%) and +// printf/absl::StrFormat placeholders (%s, %d, ...). +// The style is auto-detected; mixed styles are rejected. + +template +constexpr bool is_char_pointer_v = std::is_same_v || std::is_same_v; + +template +void streamArgument(std::ostream &os, const T &arg) { + // Use decay_t to handle potential references correctly in type checks + using ArgType = std::decay_t; + + if constexpr (std::is_same_v) { + // Do nothing - source info is collected elsewhere. + return; + } + + if constexpr (std::is_pointer_v) { + if (arg != nullptr) { + if constexpr (is_char_pointer_v) { + os << arg; + } else { + streamArgument(os, *arg); + } + } else { + os << ""; + } + } else if constexpr (Util::has_toString_v) { + os << arg.toString(); + } else if constexpr (requires { os << arg; }) { + os << arg; + } else { + throw std::runtime_error(absl::StrCat( + "Internal error: Invalid argument type in streamArgument: ", typeid(ArgType).name())); + } +} + +// Modify streamNthImpl to call the smart helper +template +void streamNthImpl(std::ostream &os, std::size_t index, const Tuple &tup, + std::index_sequence) { + bool found = ((index == Is && (streamArgument(os, std::get(tup)), true)) || ...); + if (!found) { + throw std::runtime_error(absl::StrCat( + "Internal error: Invalid argument index requested in streamNthImpl: ", index)); + } +} + +// streamNth wrapper remains the same +template +void streamNth(std::ostream &os, std::size_t index, const Tuple &tup) { + streamNthImpl(os, index, tup, std::make_index_sequence>{}); +} + +// Replicates the logic from the original getPositionTail, modifying args by reference +inline void getPositionTailLogic(const Util::SourceInfo &info, std::string &position, + std::string &tail) { + if (!info.isValid()) { + return; + } + + cstring posString = info.toPositionString(); + if (position.empty()) { + position = posString; + } else if (!posString.isNullOrEmpty()) { + // Append position only if subsequent one is found and non-empty + absl::StrAppend(&tail, "\n", posString); + } + + cstring fragment = info.toSourceFragment(); + if (!fragment.isNullOrEmpty()) { + absl::StrAppend(&tail, "\n", fragment); + } +} + +/// Iterates through tuple elements, extracts SourceInfo, updates position/tail. +template +void extractBugSourceInfoImpl(const Tuple &tup, std::string &position, std::string &tail, + std::index_sequence) { + // Use initializer list and lambda with constexpr if to process each element + (void)std::initializer_list{ + ( + [&] { + const auto &arg = std::get(tup); + using ArgType = std::decay_t; + + // Is the argument itself SourceInfo? + if constexpr (std::is_same_v) { + getPositionTailLogic(arg, position, tail); + } else if constexpr (Util::has_SourceInfo_v && + !std::is_pointer_v) { + Util::SourceInfo info = arg.getSourceInfo(); + getPositionTailLogic(info, position, tail); + } else if constexpr (std::is_pointer_v) { + using PointeeType = std::remove_pointer_t; + if constexpr (Util::has_SourceInfo_v) { + if (arg != nullptr) { + Util::SourceInfo info = arg->getSourceInfo(); + getPositionTailLogic(info, position, tail); + } + } + } + }(), + 0)... // 0 is placeholder for initializer list expansion + }; +} + +enum class FormatStyle { Unknown, Boost, Abseil, Literal }; + +inline constexpr FormatStyle detectFormatStyle(std::string_view fmt) { + bool hasBoost = false; + bool hasAbseil = false; + + for (size_t i = 0; i < fmt.size(); ++i) { + if (fmt[i] == '%') { + if (i + 1 >= fmt.size()) { + // Lone % at end - consider it Abseil style + hasAbseil = true; + if (hasBoost) return FormatStyle::Unknown; // Mixed + break; // Nothing more to parse + } + + char nextChar = fmt[i + 1]; + + if (nextChar == '%') { // Escaped %% + i++; // Skip the second % + continue; + } + + // Check specifically for Boost style: % followed by digits followed by % + if (std::isdigit(static_cast(nextChar))) { + size_t j = i + 1; // Start of digits + while (j < fmt.size() && std::isdigit(static_cast(fmt[j]))) { + j++; + } + // Check if it ends *exactly* with '%' + if (j < fmt.size() && fmt[j] == '%') { + // Found %N% pattern + hasBoost = true; + if (hasAbseil) return FormatStyle::Unknown; // Mixed style detected + i = j; // Move index past the closing '%' + continue; // Proceed to next part of the string + } + // If it was digits but didn't end in '%', it's Abseil style + // Let it fall through to the general Abseil case below. + } + + // If it wasn't %% and wasn't a valid %N% sequence, + // treat it as the start of an Abseil style specifier. + // We don't need to parse the rest of the Abseil specifier for detection. + hasAbseil = true; + if (hasBoost) return FormatStyle::Unknown; // Mixed style detected + + // We can simply skip the next character (% followed by non-digit/non-%), + // as the outer loop will increment 'i'. More complex skipping isn't needed + // just for *detection*. The actual formatting function handles parsing. + // i++; // Implicitly skipped by outer loop increment + } + // Ignore non-% characters + } + + // Final classification based on exclusively found styles + if (hasBoost && hasAbseil) return FormatStyle::Unknown; + if (hasBoost) return FormatStyle::Boost; + if (hasAbseil) return FormatStyle::Abseil; + return FormatStyle::Literal; +} + +template +std::string formatBoostStyle(std::string_view formatSv, const Tuple &argTuple) { + std::stringstream ss; + constexpr std::size_t numArgs = std::tuple_size_v; + std::size_t currentPos = 0; + + while (currentPos < formatSv.size()) { + size_t nextPercent = formatSv.find('%', currentPos); + if (nextPercent == std::string_view::npos) { + ss << formatSv.substr(currentPos); + break; + } + ss << formatSv.substr(currentPos, nextPercent - currentPos); + currentPos = nextPercent; + + if (currentPos + 1 >= formatSv.size()) { + ss << '%'; + currentPos++; + continue; + } + + if (formatSv[currentPos + 1] == '%') { // %% + ss << '%'; + currentPos += 2; + } else if (std::isdigit(static_cast(formatSv[currentPos + 1]))) { // %N% + size_t j = currentPos + 1; + while (j < formatSv.size() && std::isdigit(static_cast(formatSv[j]))) { + ++j; + } + if (j < formatSv.size() && formatSv[j] == '%') { + std::string_view numSv = formatSv.substr(currentPos + 1, j - (currentPos + 1)); + uint64_t argIndexBoost = 0; + auto result = + std::from_chars(numSv.data(), numSv.data() + numSv.size(), argIndexBoost); + if (result.ec == std::errc() && argIndexBoost > 0 && argIndexBoost <= numArgs) { + detail::streamNth(ss, static_cast(argIndexBoost - 1), argTuple); + currentPos = j + 1; + } else { + ss << '%'; + currentPos++; + } // Invalid N + } else { + ss << '%'; + currentPos++; + // %digits without % + } + } else { + // % followed by non-% non-digit + ss << '%'; + currentPos++; + } + } + return ss.str(); +} + +template // Is is the pack 0, 1, 2... +void populateSelectiveAbseilArgsImpl(const Tuple &tup, std::vector &argStrings, + std::vector &abslArgs, + std::index_sequence) { + abslArgs.reserve(std::tuple_size_v); + argStrings.reserve(std::tuple_size_v); + + // Use initializer list and lambda with constexpr if to process each element. + (void)std::initializer_list{ + ( + [&] { + const auto &element = std::get(tup); + using ElementType = std::decay_t; + + if constexpr (std::is_integral_v) { + // Pass integral natively. + abslArgs.push_back(absl::FormatArg(element)); + } else { + // Convert other types to string. + std::stringstream tempSs; + detail::streamArgument(tempSs, element); + argStrings.push_back(tempSs.str()); + abslArgs.push_back(absl::FormatArg(argStrings.back())); + } + }(), + 0)... // 0 is placeholder for initializer list expansion + }; +} + +// Wrapper to generate the index sequence +template +void populateSelectiveAbseilArgs(const Tuple &tup, std::vector &argStrings, + std::vector &abslArgs) { + populateSelectiveAbseilArgsImpl(tup, argStrings, abslArgs, + std::make_index_sequence>{}); +} + +template +std::string formatAbslStyle(std::string_view formatSv, const Tuple &argTuple) { + // 1. Populate abslArgs vector: integrals passed directly, others as strings + std::vector argStrings; + std::vector abslArgs; + populateSelectiveAbseilArgs(argTuple, argStrings, abslArgs); + + // 2. Call Abseil Format + std::string result; + absl::UntypedFormatSpec spec(formatSv); + if (!absl::FormatUntyped(&result, spec, abslArgs)) { + // Formatting might fail if specifier doesn't match type. + // e.g., %d applied to a pre-rendered string or the "" placeholder. + // Or %s applied to an integral (Abseil might handle this, though). + throw std::runtime_error( + absl::StrCat("absl::Format error. Can not format with the provided arguments")); + } + return result; +} + +} // namespace detail + +template +void extractBugSourceInfo(const Tuple &tup, std::string &position, std::string &tail) { + detail::extractBugSourceInfoImpl(tup, position, tail, + std::make_index_sequence>{}); +} + +template +std::string createFormattedMessageFromTuple(const char *format, const Tuple &argTuple) { + std::string_view formatSv(format); + detail::FormatStyle style = detail::detectFormatStyle(formatSv); + + try { + switch (style) { + case detail::FormatStyle::Boost: + return detail::formatBoostStyle(formatSv, argTuple); + case detail::FormatStyle::Abseil: + // Pass the original tuple to the Abseil formatter. + return detail::formatAbslStyle(formatSv, argTuple); + case detail::FormatStyle::Literal: + return std::string(formatSv); + // Mixed or invalid style + case detail::FormatStyle::Unknown: + default: + throw std::runtime_error("Can not determine appropriate format."); + } + } catch (const std::exception &e) { + return absl::StrCat("<>"); + } catch (...) { + return absl::StrCat("<>"); + } +} + +template +std::string createFormattedMessage(const char *format, Args &&...args) { + auto argTuple = std::make_tuple(std::forward(args)...); + return createFormattedMessageFromTuple(format, argTuple); +} + +} // namespace P4 + +#endif /* LIB_BOOST_FORMAT_HELPER_H_ */ diff --git a/lib/bug_helper.h b/lib/bug_helper.h index 83d23fcf748..55289a35912 100644 --- a/lib/bug_helper.h +++ b/lib/bug_helper.h @@ -17,133 +17,25 @@ limitations under the License. #ifndef LIB_BUG_HELPER_H_ #define LIB_BUG_HELPER_H_ -#include -#include #include #include +#include #include -#include - #include "absl/strings/str_cat.h" -#include "cstring.h" -#include "source_file.h" -#include "stringify.h" +#include "lib/boost_format_helper.h" namespace P4 { -namespace detail { - -static inline std::pair getPositionTail(const Util::SourceInfo &info, - std::string_view position, - std::string_view tail) { - std::string_view posString = info.toPositionString(); - std::string outTail(tail); - if (position.empty()) { - position = posString; - } else { - outTail.append(posString); - if (!posString.empty()) outTail.append("\n"); - } - outTail += info.toSourceFragment(); - - return std::pair(position, outTail); -} - -template -std::pair maybeAddSourceInfo(const T &t, std::string_view position, - std::string_view tail) { - if constexpr (Util::has_SourceInfo_v) - return getPositionTail(t.getSourceInfo(), position, tail); - - (void)position; - (void)tail; - return {"", ""}; -} - -static inline std::string bug_helper(boost::format &f, std::string_view position, - std::string_view tail) { - return absl::StrCat(position, position.empty() ? "" : ": ", boost::str(f), "\n", tail); -} - -template -auto bug_helper(boost::format &f, std::string_view position, std::string_view tail, const T *t, - Args &&...args); - -template -auto bug_helper(boost::format &f, std::string_view position, std::string_view tail, const T &t, - Args &&...args) -> std::enable_if_t, std::string>; - -template -std::string bug_helper(boost::format &f, std::string_view position, std::string_view tail, - const char *t, Args &&...args) { - return bug_helper(f % t, position, tail, std::forward(args)...); -} - -template -std::string bug_helper(boost::format &f, std::string_view position, std::string_view tail, - const Util::SourceInfo &info, Args &&...args) { - auto [outPos, outTail] = detail::getPositionTail(info, position, tail); - return bug_helper(f % "", outPos, outTail, std::forward(args)...); -} - -template -struct DbprintDispatchPtr { - const T *val; -}; - -template -std::ostream &operator<<(std::ostream &os, const DbprintDispatchPtr &dispatch) { - if constexpr (has_dbprint_v) { - dispatch.val->dbprint(os); - } else { - static_assert(has_ostream_operator_v, - "cannot debug print this type, implement dbprint method"); - os << dispatch.val; - } - - return os; -} - -template -auto bug_helper(boost::format &f, std::string_view position, std::string_view tail, const T *t, - Args &&...args) { - if (t == nullptr) return bug_helper(f, position, tail, std::forward(args)...); - - auto [outPos, outTail] = maybeAddSourceInfo(*t, position, tail); - return bug_helper(f % DbprintDispatchPtr{t}, outPos, outTail, std::forward(args)...); -} - -template -struct DbprintDispatchRef { - const T &val; -}; - -template -std::ostream &operator<<(std::ostream &os, const DbprintDispatchRef &dispatch) { - if constexpr (has_dbprint_v) { - dispatch.val.dbprint(os); - } else { - static_assert(has_ostream_operator_v, - "cannot debug print this type, implement dbprint method"); - os << dispatch.val; - } - - return os; -} - -template -auto bug_helper(boost::format &f, std::string_view position, std::string_view tail, const T &t, - Args &&...args) -> std::enable_if_t, std::string> { - auto [outPos, outTail] = maybeAddSourceInfo(t, position, tail); - return bug_helper(f % DbprintDispatchRef{t}, outPos, outTail, std::forward(args)...); -} -} // namespace detail -// Most direct invocations of bug_helper usually only reduce arguments template -std::string bug_helper(boost::format &f, std::string_view position, std::string_view tail, +std::string bug_helper(const char *format, std::string_view position, std::string_view tail, Args &&...args) { - return detail::bug_helper(f, position, tail, std::forward(args)...); + auto argTuple = std::forward_as_tuple(args...); + std::string positionStr(position); + std::string tailStr(tail); + extractBugSourceInfo(argTuple, positionStr, tailStr); + const std::string formatted = createFormattedMessageFromTuple(format, argTuple); + return absl::StrCat(positionStr, positionStr.empty() ? "" : ": ", formatted, "\n", tailStr); } } // namespace P4 diff --git a/lib/error.h b/lib/error.h index 9808b009377..8d2347f133a 100644 --- a/lib/error.h +++ b/lib/error.h @@ -47,9 +47,8 @@ inline unsigned diagnosticCount() { return BaseCompileContext::get().errorReporter().getDiagnosticCount(); } -// Errors, warnings, and infos are specified using boost::format format strings, i.e., -// %1%, %2%, etc (starting at 1, not at 0). -// Some compatibility for printf-style arguments is also supported. +// Errors, warnings, and infos support boost-style placeholders (%1%, %2%, ...) +// and printf/absl::StrFormat-style placeholders (%s, %d, ...). /// Report an error with the given message. // LEGACY: once we transition to error types, this should be deprecated diff --git a/lib/error_helper.h b/lib/error_helper.h index bedf17f09e7..39cd5dbbd20 100644 --- a/lib/error_helper.h +++ b/lib/error_helper.h @@ -17,9 +17,9 @@ limitations under the License. #define LIB_ERROR_HELPER_H_ #include +#include -#include - +#include "lib/boost_format_helper.h" #include "lib/error_message.h" #include "lib/source_file.h" #include "lib/stringify.h" @@ -27,83 +27,67 @@ limitations under the License. namespace P4 { namespace priv { -// All these methods return std::string because this is the native format of boost::format -// Position is printed at the beginning. -static inline ErrorMessage error_helper(boost::format &f, ErrorMessage out) { - out.message = boost::str(f); - return out; -} - -template -auto error_helper(boost::format &f, ErrorMessage out, const char *t, Args &&...args) { - return error_helper(f % t, out, std::forward(args)...); -} - -template -auto error_helper(boost::format &f, ErrorMessage out, const T &t, - Args &&...args) -> std::enable_if_t, ErrorMessage>; - -template -auto error_helper(boost::format &f, ErrorMessage out, const T &t, Args &&...args) - -> std::enable_if_t && !std::is_pointer_v, ErrorMessage>; - -template -auto error_helper(boost::format &f, ErrorMessage out, const T *t, Args &&...args) { - // Contrary to bug_helper we do not want to show raw pointers to users in - // ordinary error messages. Therefore we explicitly delegate to - // reference-arg implementation here. - return error_helper(f, out, *t, std::forward(args)...); -} - -template -ErrorMessage error_helper(boost::format &f, ErrorMessage out, const Util::SourceInfo &info, - Args &&...args) { - if (info.isValid()) out.locations.push_back(info); - return error_helper(f % "", std::move(out), std::forward(args)...); -} - -template -void maybeAddSourceInfo(ErrorMessage &out, const T &t) { - if constexpr (Util::has_SourceInfo_v) { - auto info = t.getSourceInfo(); - if (info.isValid()) out.locations.push_back(info); +inline void collectSourceInfo(ErrorMessage & /*msg*/) {} + +template +void collectSourceInfo(ErrorMessage &msg, T &&arg, Rest &&...rest) { + using RawType = std::remove_reference_t; + using ArgType = std::decay_t; + if constexpr (Util::has_SourceInfo_v) { + if constexpr (std::is_convertible_v) { + Util::SourceInfo info(arg); + if (info.isValid()) { + msg.locations.push_back(info); + } + } else { + auto info = arg.getSourceInfo(); + if (info.isValid()) { + msg.locations.push_back(info); + } + } + } else if constexpr (std::is_array_v && + std::is_same_v>, char>) { + // String literal or char array: not a nullable pointer source. + } else if constexpr (std::is_pointer_v) { + using PointeeType = std::remove_pointer_t; + if constexpr (Util::has_SourceInfo_v) { + if (arg != nullptr) { + auto info = arg->getSourceInfo(); + if (info.isValid()) { + msg.locations.push_back(info); + } + } + } + } else if constexpr (std::is_same_v) { + if (arg.isValid()) { + msg.locations.push_back(arg); + } } -} - -template -auto error_helper(boost::format &f, ErrorMessage out, const T &t, Args &&...args) - -> std::enable_if_t && !std::is_pointer_v, ErrorMessage> { - maybeAddSourceInfo(out, t); - return error_helper(f % t, std::move(out), std::forward(args)...); -} - -template -auto error_helper(boost::format &f, ErrorMessage out, const T &t, - Args &&...args) -> std::enable_if_t, ErrorMessage> { - maybeAddSourceInfo(out, t); - return error_helper(f % t.toString(), std::move(out), std::forward(args)...); + collectSourceInfo(msg, std::forward(rest)...); } } // namespace priv // Most direct invocations of error_helper usually only reduce arguments template -ErrorMessage error_helper(boost::format &f, Args &&...args) { +ErrorMessage error_helper(const char *format, Args &&...args) { ErrorMessage msg; - return priv::error_helper(f, msg, std::forward(args)...); + return error_helper(format, std::move(msg), std::forward(args)...); } // Invoked from ErrorReporter template -ErrorMessage error_helper(boost::format &f, ErrorMessage msg, Args &&...args) { - return priv::error_helper(f, std::move(msg), std::forward(args)...); +ErrorMessage error_helper(const char *format, ErrorMessage msg, Args &&...args) { + priv::collectSourceInfo(msg, args...); + msg.message = createFormattedMessage(format, std::forward(args)...); + return msg; } // This overload exists for backwards compatibility template -ErrorMessage error_helper(boost::format &f, const std::string &prefix, const Util::SourceInfo &info, - const std::string &suffix, Args &&...args) { - return priv::error_helper(f, ErrorMessage(prefix, info, suffix), std::forward(args)...); +ErrorMessage error_helper(const char *format, const std::string &prefix, + const Util::SourceInfo &info, const std::string &suffix, Args &&...args) { + return error_helper(format, ErrorMessage(prefix, info, suffix), std::forward(args)...); } } // namespace P4 diff --git a/lib/error_message.h b/lib/error_message.h index 2d09e2cc13e..ca057801a59 100644 --- a/lib/error_message.h +++ b/lib/error_message.h @@ -27,8 +27,8 @@ namespace P4 { * Structure populated via error_helper functions * * Typically, calls to ::P4::error/::P4::warning have many parameters, some of them - * might have SourceInfo attribute. ::P4::error_helper parse those parameters, format - * parameters to output message and extracts SourceInfo wherever possible. + * might have SourceInfo. ::P4::error_helper parses those parameters, formats the + * message, and extracts SourceInfo wherever possible. * * Populated structure can be serialized to canonical error message with toString() method. * diff --git a/lib/error_reporter.h b/lib/error_reporter.h index 4bceac7dba1..f8eb129e878 100644 --- a/lib/error_reporter.h +++ b/lib/error_reporter.h @@ -22,14 +22,15 @@ limitations under the License. #include #include #include +#include -#include - +#include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" -#include "bug_helper.h" #include "error_catalog.h" -#include "error_helper.h" +#include "error_message.h" #include "exceptions.h" +#include "lib/boost_format_helper.h" +#include "lib/source_file.h" namespace P4 { @@ -41,11 +42,49 @@ enum class DiagnosticAction { Error /// Print an error and signal that compilation should be aborted. }; +namespace detail { +// Base case +inline void collectSourceInfo(ErrorMessage & /*msg*/) {} +// Recursive step +template +void collectSourceInfo(ErrorMessage &msg, T &&arg, Rest &&...rest) { + using ArgType = std::decay_t; + if constexpr (Util::has_SourceInfo_v) { + if constexpr (std::is_convertible_v) { + Util::SourceInfo info(arg); + if (info.isValid()) { + msg.locations.push_back(info); + } + } else { + auto info = arg.getSourceInfo(); + if (info.isValid()) { + msg.locations.push_back(info); + } + } + } else if constexpr (std::is_pointer_v) { + using PointeeType = std::remove_pointer_t; + if constexpr (Util::has_SourceInfo_v) { + if (arg != nullptr) { + auto info = arg->getSourceInfo(); + if (info.isValid()) { + msg.locations.push_back(info); + } + } + } + } else if constexpr (std::is_same_v) { + if (arg.isValid()) { + msg.locations.push_back(arg); + } + } + + collectSourceInfo(msg, std::forward(rest)...); +} +} // namespace detail + // Keeps track of compilation errors. -// Errors are specified using the error() and warning() methods, -// that use boost::format format strings, i.e., -// %1%, %2%, etc (starting at 1, not at 0). -// Some compatibility for printf-style arguments is also supported. +// Errors are specified using the error() and warning() methods. +// Supported formatting styles are boost-style (%1%, %2%, ...) and +// printf/absl::StrFormat-style (%s, %d, ...). class ErrorReporter { protected: unsigned int infoCount; @@ -88,26 +127,29 @@ class ErrorReporter { warningCount(0), errorCount(0), maxErrorCount(20), + outputstream(&std::cerr), defaultInfoDiagnosticAction(DiagnosticAction::Info), defaultWarningDiagnosticAction(DiagnosticAction::Warn) { - outputstream = &std::cerr; ErrorCatalog::getCatalog().initReporter(*this); } virtual ~ErrorReporter() = default; - // error message for a bug + // Error message for a bug. template std::string bug_message(const char *format, Args &&...args) { - boost::format fmt(format); - // FIXME: This will implicitly take location of the first argument having - // SourceInfo. Not sure if this always desireable or not. - return ::P4::bug_helper(fmt, "", "", std::forward(args)...); + auto argTuple = std::forward_as_tuple(args...); + std::string positionStr; + std::string tailStr; + extractBugSourceInfo(argTuple, positionStr, tailStr); + const std::string formattedCore = P4::createFormattedMessageFromTuple(format, argTuple); + return absl::StrCat(positionStr, positionStr.empty() ? "" : ": ", formattedCore, "\n", + tailStr); } + // Formats a message string directly. Does not attach SourceInfo metadata. template std::string format_message(const char *format, Args &&...args) { - boost::format fmt(format); - return ::P4::error_helper(fmt, std::forward(args)...).toString(); + return P4::createFormattedMessage(format, std::forward(args)...); } template ()->getSourceInfo()), typename... Args> @@ -159,9 +201,9 @@ class ErrorReporter { msgType = ErrorMessage::MessageType::Error; } - boost::format fmt(format); ErrorMessage msg(msgType, diagnosticName ? diagnosticName : "", suffix); - msg = ::P4::error_helper(fmt, msg, std::forward(args)...); + detail::collectSourceInfo(msg, std::forward(args)...); + msg.message = P4::createFormattedMessage(format, std::forward(args)...); emit_message(msg); if (errorCount > maxErrorCount) diff --git a/lib/exceptions.h b/lib/exceptions.h index 17635bfa9e5..28975e54a64 100644 --- a/lib/exceptions.h +++ b/lib/exceptions.h @@ -22,16 +22,15 @@ limitations under the License. #include #include - -#include +#include // +#include #include "absl/strings/str_cat.h" -#include "lib/bug_helper.h" +#include "lib/boost_format_helper.h" namespace P4::Util { // colors to pretty print messages -// \e is non-standard escape sequence, use codepoint \33 instead constexpr char ANSI_RED[] = "\33[31m"; constexpr char ANSI_BLUE[] = "\33[34m"; constexpr char ANSI_CLR[] = "\33[0m"; @@ -65,8 +64,8 @@ inline const char *cerr_clear_colors() { } /// Base class for all exceptions. -/// The constructor uses boost::format for the format string, i.e., -/// %1%, %2%, etc (starting at 1, not at 0) +/// Supports boost-style (%N%) and printf/absl::StrFormat-style (%s, %d, etc.) +/// placeholders. class P4CExceptionBase : public std::exception { protected: std::string message; @@ -76,13 +75,21 @@ class P4CExceptionBase : public std::exception { template explicit P4CExceptionBase(const char *format, Args &&...args) { traceCreation(); - boost::format fmt(format); - // FIXME: This will implicitly take location of the first argument having - // SourceInfo. Not sure if this always desireable or not. - message = ::P4::bug_helper(fmt, "", "", std::forward(args)...); + + // Capture arguments once and reuse the tuple to avoid double-moving rvalues. + auto argTuple = std::forward_as_tuple(args...); + // Extract SourceInfo details. + std::string positionStr; + std::string tailStr; + extractBugSourceInfo(argTuple, positionStr, tailStr); + // Format the core message. + std::string formattedCore = P4::createFormattedMessageFromTuple(format, argTuple); + // Assemble the final message string. + message = absl::StrCat(positionStr, positionStr.empty() ? "" : ": ", formattedCore, "\n", + tailStr); } - const char *what() const noexcept override { return message.c_str(); } + [[nodiscard]] const char *what() const noexcept override { return message.c_str(); } }; /// This class indicates a bug in the compiler diff --git a/test/gtest/exception_test.cpp b/test/gtest/exception_test.cpp index 43d3c3799c6..f3d395a1636 100644 --- a/test/gtest/exception_test.cpp +++ b/test/gtest/exception_test.cpp @@ -30,7 +30,7 @@ TEST(UtilException, Messages) { } catch (std::exception &ex) { cstring err(ex.what()); cstring redir_msg("Compiler Bug:\ntest\n"); - cstring no_redir_msg = cstring(ANSI_RED) + "Compiler Bug" + ANSI_CLR + ":\ntest\n"; + cstring no_redir_msg = cstring(ANSI_RED) + "Compiler Bug" + ANSI_CLR + ":\ntest"; // The error message might or might not be colorized based on if the test are redirected // or not, to make sure both options are valid an array is used. bool is_content_correct = (err == redir_msg) || (err == no_redir_msg); diff --git a/test/gtest/format_test.cpp b/test/gtest/format_test.cpp index 3b8759cde03..e71a8cac8ce 100644 --- a/test/gtest/format_test.cpp +++ b/test/gtest/format_test.cpp @@ -17,7 +17,6 @@ limitations under the License. #include #include "lib/cstring.h" -#include "lib/error.h" #include "lib/stringify.h" namespace P4::Util { @@ -25,13 +24,13 @@ namespace P4::Util { TEST(Util, Format) { auto &context = BaseCompileContext::get(); cstring message = context.errorReporter().format_message("%1%", 5u); - EXPECT_EQ("5\n", message); + EXPECT_EQ("5", message); message = context.errorReporter().format_message("Number=%1%", 5); - EXPECT_EQ("Number=5\n", message); + EXPECT_EQ("Number=5", message); message = context.errorReporter().format_message("Double=%1% String=%2%", 2.3, "short"); - EXPECT_EQ("Double=2.3 String=short\n", message); + EXPECT_EQ("Double=2.3 String=short", message); struct NiceFormat { int a, b, c; @@ -44,15 +43,15 @@ TEST(Util, Format) { NiceFormat nf{1, 2, 3}; message = context.errorReporter().format_message("Nice=%1%", nf); - EXPECT_EQ("Nice=(1,2,3)\n", message); + EXPECT_EQ("Nice=(1,2,3)", message); cstring x = "x"_cs; cstring y = "y"_cs; message = context.errorReporter().format_message("%1% %2%", x, y); - EXPECT_EQ("x y\n", message); + EXPECT_EQ("x y", message); message = context.errorReporter().format_message("%1% %2%", x, 5); - EXPECT_EQ("x 5\n", message); + EXPECT_EQ("x 5", message); } } // namespace P4::Util diff --git a/test/gtest/source_file_test.cpp b/test/gtest/source_file_test.cpp index cca36661291..ad43794752c 100644 --- a/test/gtest/source_file_test.cpp +++ b/test/gtest/source_file_test.cpp @@ -33,7 +33,7 @@ TEST(UtilSourceFile, SourcePosition) { auto &context = BaseCompileContext::get(); cstring str = context.errorReporter().format_message("%1% - %2%", position, position); - EXPECT_EQ("3:3 - 3:3\n", str); + EXPECT_EQ("3:3 - 3:3", str); SourcePosition next(3, 4); EXPECT_LT(position, next);