Skip to content

Commit

Permalink
Improve BUG_CHECK internals (p4lang#4678)
Browse files Browse the repository at this point in the history
* Refactor slightly bug_helper and corresponding code:
 - Ensure we are using forwarding references and do not pass things by value
 - Use std::string instead of cstring to hold full message inside exception,
   cstring does not make any sense here
 - Switch to std::string_view to capture string arguments instead of std::string
   passed by value

* Use abseil string library to simplify the code

* No need to special case `big_int`, it will be handled by the generic template code

* Get rid of `message` argument, it is always empty

* Unbreak bazel build

* Address review comments
  • Loading branch information
asl authored May 27, 2024
1 parent d504a13 commit fa68263
Show file tree
Hide file tree
Showing 7 changed files with 183 additions and 183 deletions.
1 change: 1 addition & 0 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ cc_library(
"@boost//:format",
"@boost//:multiprecision",
"@com_google_absl//absl/numeric:bits",
"@com_google_absl//absl/strings",
"@com_google_googletest//:gtest",
],
)
Expand Down
1 change: 1 addition & 0 deletions lib/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -98,5 +98,6 @@ set_target_properties(p4ctoolkit PROPERTIES COMPILE_FLAGS -fno-builtin)
target_link_libraries(p4ctoolkit
# These libraries are exposed by a header.
PUBLIC absl::bits
PUBLIC absl::strings
PUBLIC ${LIBGC_LIBRARIES}
)
142 changes: 142 additions & 0 deletions lib/bug_helper.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
/*
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

/* -*-c++-*- */

#ifndef LIB_BUG_HELPER_H_
#define LIB_BUG_HELPER_H_

#include <string>
#include <string_view>

#include <boost/format.hpp>

#include "absl/strings/str_cat.h"
#include "lib/cstring.h"
#include "lib/source_file.h"

template <class... Args>
std::string bug_helper(boost::format &f, std::string_view position, std::string_view tail,
const char *t, Args &&...args);

template <class... Args>
std::string bug_helper(boost::format &f, std::string_view position, std::string_view tail,
const cstring &t, Args &&...args);

template <class... Args>
std::string bug_helper(boost::format &f, std::string_view position, std::string_view tail,
const Util::SourceInfo &info, Args &&...args);

template <typename T, class... Args>
auto bug_helper(boost::format &f, std::string_view position, std::string_view tail, const T &t,
Args &&...args)
-> std::enable_if_t<std::is_base_of_v<Util::IHasSourceInfo, T>, std::string>;

template <typename T, class... Args>
auto bug_helper(boost::format &f, std::string_view position, std::string_view tail, const T &t,
Args &&...args)
-> std::enable_if_t<!std::is_base_of_v<Util::IHasSourceInfo, T>, std::string>;

template <typename T, class... Args>
auto bug_helper(boost::format &f, std::string_view position, std::string_view tail, const T *t,
Args &&...args)
-> std::enable_if_t<std::is_base_of_v<Util::IHasSourceInfo, T>, std::string>;

template <typename T, class... Args>
auto bug_helper(boost::format &f, std::string_view position, std::string_view tail, const T *t,
Args &&...args)
-> std::enable_if_t<!std::is_base_of_v<Util::IHasSourceInfo, T>, std::string>;

// actual implementations
namespace detail {
static inline auto getPositionTail(const Util::SourceInfo &info, std::string_view position,
std::string_view tail) {
std::string_view posString = info.toPositionString().string_view();
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);
}
} // namespace detail

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 <class... Args>
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>(args)...);
}

template <class... Args>
std::string bug_helper(boost::format &f, std::string_view position, std::string_view tail,
const cstring &t, Args &&...args) {
return bug_helper(f % t.string_view(), position, tail, std::forward<Args>(args)...);
}

template <typename T, class... Args>
auto bug_helper(boost::format &f, std::string_view position, std::string_view tail, const T *t,
Args &&...args)
-> std::enable_if_t<!std::is_base_of_v<Util::IHasSourceInfo, T>, std::string> {
std::stringstream str;
str << t;
return bug_helper(f % str.str(), position, tail, std::forward<Args>(args)...);
}

template <typename T, class... Args>
auto bug_helper(boost::format &f, std::string_view position, std::string_view tail, const T &t,
Args &&...args)
-> std::enable_if_t<!std::is_base_of_v<Util::IHasSourceInfo, T>, std::string> {
return bug_helper(f % t, position, tail, std::forward<Args>(args)...);
}

template <class... Args>
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>(args)...);
}

template <typename T, class... Args>
auto bug_helper(boost::format &f, std::string_view position, std::string_view tail, const T *t,
Args &&...args)
-> std::enable_if_t<std::is_base_of_v<Util::IHasSourceInfo, T>, std::string> {
if (t == nullptr) return bug_helper(f, position, tail, std::forward<Args>(args)...);

auto [outPos, outTail] = detail::getPositionTail(t->getSourceInfo(), position, tail);
std::stringstream str;
str << t;
return bug_helper(f % str.str(), outPos, outTail, std::forward<Args>(args)...);
}

template <typename T, class... Args>
auto bug_helper(boost::format &f, std::string_view position, std::string_view tail, const T &t,
Args &&...args)
-> std::enable_if_t<std::is_base_of_v<Util::IHasSourceInfo, T>, std::string> {
auto [outPos, outTail] = detail::getPositionTail(t.getSourceInfo(), position, tail);

std::stringstream str;
str << t;
return bug_helper(f % str.str(), outPos, outTail, std::forward<Args>(args)...);
}

#endif /* LIB_BUG_HELPER_H_ */
153 changes: 0 additions & 153 deletions lib/error_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,157 +165,4 @@ ErrorMessage error_helper(boost::format &f, const std::string &prefix, const Uti
return ::priv::error_helper(f, msg, std::forward<Args>(args)...);
}

/***********************************************************************************/

static inline std::string bug_helper(boost::format &f, std::string message, std::string position,
std::string tail) {
std::string text = boost::str(f);
std::string result = position;
if (!position.empty()) result += ": ";
result += message + text + "\n" + tail;
return result;
}

template <class... Args>
std::string bug_helper(boost::format &f, std::string message, std::string position,
std::string tail, const char *t, Args... args);

template <class... Args>
std::string bug_helper(boost::format &f, std::string message, std::string position,
std::string tail, const cstring &t, Args... args);

template <class... Args>
std::string bug_helper(boost::format &f, std::string message, std::string position,
std::string tail, const Util::SourceInfo &info, Args... args);

template <typename T, class... Args>
auto bug_helper(boost::format &f, std::string message, std::string position, std::string tail,
const T &t, Args... args) ->
typename std::enable_if<std::is_base_of<Util::IHasSourceInfo, T>::value, std::string>::type;

template <typename T, class... Args>
auto bug_helper(boost::format &f, std::string message, std::string position, std::string tail,
const T *t, Args... args) ->
typename std::enable_if<std::is_base_of<Util::IHasSourceInfo, T>::value, std::string>::type;

template <typename T, class... Args>
auto bug_helper(boost::format &f, std::string message, std::string position, std::string tail,
const T *t, Args... args) ->
typename std::enable_if<!std::is_base_of<Util::IHasSourceInfo, T>::value, std::string>::type;

template <class... Args>
std::string bug_helper(boost::format &f, std::string message, std::string position,
std::string tail, const big_int *t, Args... args);

template <class... Args>
std::string bug_helper(boost::format &f, std::string message, std::string position,
std::string tail, const big_int &t, Args... args);

template <typename T, class... Args>
auto bug_helper(boost::format &f, std::string message, std::string position, std::string tail,
const T &t, Args... args) ->
typename std::enable_if<!std::is_base_of<Util::IHasSourceInfo, T>::value, std::string>::type;

// actual implementations

template <class... Args>
std::string bug_helper(boost::format &f, std::string message, std::string position,
std::string tail, const char *t, Args... args) {
return bug_helper(f % t, message, position, tail, std::forward<Args>(args)...);
}

template <class... Args>
std::string bug_helper(boost::format &f, std::string message, std::string position,
std::string tail, const cstring &t, Args... args) {
return bug_helper(f % t.c_str(), message, position, tail, std::forward<Args>(args)...);
}

template <typename T, class... Args>
auto bug_helper(boost::format &f, std::string message, std::string position, std::string tail,
const T *t, Args... args) ->
typename std::enable_if<!std::is_base_of<Util::IHasSourceInfo, T>::value, std::string>::type {
std::stringstream str;
str << t;
return bug_helper(f % str.str(), message, position, tail, std::forward<Args>(args)...);
}

template <class... Args>
std::string bug_helper(boost::format &f, std::string message, std::string position,
std::string tail, const big_int *t, Args... args) {
return bug_helper(f % t, message, position, tail, std::forward<Args>(args)...);
}

template <class... Args>
std::string bug_helper(boost::format &f, std::string message, std::string position,
std::string tail, const big_int &t, Args... args) {
return bug_helper(f % t, message, position, tail, std::forward<Args>(args)...);
}

template <typename T, class... Args>
auto bug_helper(boost::format &f, std::string message, std::string position, std::string tail,
const T &t, Args... args) ->
typename std::enable_if<!std::is_base_of<Util::IHasSourceInfo, T>::value, std::string>::type {
return bug_helper(f % t, message, position, tail, std::forward<Args>(args)...);
}

template <class... Args>
std::string bug_helper(boost::format &f, std::string message, std::string position,
std::string tail, const Util::SourceInfo &info, Args... args) {
cstring posString = info.toPositionString();
if (position.empty()) {
position = posString;
posString = "";
} else {
if (!posString.isNullOrEmpty()) {
posString += "\n";
}
}
return bug_helper(f % "", message, position, tail + posString + info.toSourceFragment(),
std::forward<Args>(args)...);
}

template <typename T, class... Args>
auto bug_helper(boost::format &f, std::string message, std::string position, std::string tail,
const T *t, Args... args) ->
typename std::enable_if<std::is_base_of<Util::IHasSourceInfo, T>::value, std::string>::type {
if (t == nullptr) {
return bug_helper(f, message, position, tail, std::forward<Args>(args)...);
}

cstring posString = t->getSourceInfo().toPositionString();
if (position.empty()) {
position = posString;
posString = "";
} else {
if (!posString.isNullOrEmpty()) {
posString += "\n";
}
}
std::stringstream str;
str << t;
return bug_helper(f % str.str(), message, position,
tail + posString + t->getSourceInfo().toSourceFragment(),
std::forward<Args>(args)...);
}

template <typename T, class... Args>
auto bug_helper(boost::format &f, std::string message, std::string position, std::string tail,
const T &t, Args... args) ->
typename std::enable_if<std::is_base_of<Util::IHasSourceInfo, T>::value, std::string>::type {
cstring posString = t.getSourceInfo().toPositionString();
if (position.empty()) {
position = posString;
posString = "";
} else {
if (!posString.isNullOrEmpty()) {
posString += "\n";
}
}
std::stringstream str;
str << t;
return bug_helper(f % str.str(), message, position,
tail + posString + t.getSourceInfo().toSourceFragment(),
std::forward<Args>(args)...);
}

#endif /* LIB_ERROR_HELPER_H_ */
10 changes: 6 additions & 4 deletions lib/error_reporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ limitations under the License.

#include <boost/format.hpp>

#include "bug_helper.h"
#include "error_catalog.h"
#include "error_helper.h"
#include "exceptions.h"
Expand Down Expand Up @@ -91,11 +92,12 @@ class ErrorReporter {
}

// error message for a bug
template <typename... T>
std::string bug_message(const char *format, T... args) {
template <typename... Args>
std::string bug_message(const char *format, Args &&...args) {
boost::format fmt(format);
std::string message = ::bug_helper(fmt, "", "", "", args...);
return message;
// FIXME: This will implicitly take location of the first argument having
// SourceInfo. Not sure if this always desireable or not.
return ::bug_helper(fmt, "", "", std::forward<Args>(args)...);
}

template <typename... T>
Expand Down
Loading

0 comments on commit fa68263

Please sign in to comment.