Skip to content

Commit fc15ca8

Browse files
ZedThreeZedThree
authored
Merge pull request #3224 from boutproject/forward-exception-args
Forward arguments to `BoutException` and `Output::write`
2 parents 3aa689d + ab61b9e commit fc15ca8

File tree

6 files changed

+111
-69
lines changed

6 files changed

+111
-69
lines changed

‎include/bout/boutexception.hxx‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,16 @@ void BoutParallelThrowRhsFail(int status, const char* message);
1616

1717
class BoutException : public std::exception {
1818
public:
19+
BoutException(const BoutException&) = default;
20+
BoutException(BoutException&&) = delete;
21+
BoutException& operator=(const BoutException&) = default;
22+
BoutException& operator=(BoutException&&) = delete;
1923
BoutException(std::string msg);
2024

2125
template <class S, class... Args>
22-
BoutException(const S& format, const Args&... args)
23-
: BoutException(fmt::format(format, args...)) {}
26+
BoutException(S&& format, Args&&... args)
27+
: BoutException(fmt::format(std::forward<S>(format),
28+
std::forward<decltype(args)>(args)...)) {}
2429

2530
~BoutException() override;
2631

@@ -38,24 +43,24 @@ private:
3843
int trace_size;
3944
char** messages;
4045
#endif
41-
42-
void makeBacktrace();
4346
};
4447

4548
class BoutRhsFail : public BoutException {
4649
public:
4750
BoutRhsFail(std::string message) : BoutException(std::move(message)) {}
4851
template <class S, class... Args>
49-
BoutRhsFail(const S& format, const Args&... args)
50-
: BoutRhsFail(fmt::format(format, args...)) {}
52+
BoutRhsFail(S&& format, Args&&... args)
53+
: BoutRhsFail(fmt::format(std::forward<S>(format),
54+
std::forward<decltype(args)>(args)...)) {}
5155
};
5256

5357
class BoutIterationFail : public BoutException {
5458
public:
5559
BoutIterationFail(std::string message) : BoutException(std::move(message)) {}
5660
template <class S, class... Args>
57-
BoutIterationFail(const S& format, const Args&... args)
58-
: BoutIterationFail(fmt::format(format, args...)) {}
61+
BoutIterationFail(S&& format, Args&&... args)
62+
: BoutIterationFail(fmt::format(std::forward<S>(format),
63+
std::forward<decltype(args)>(args)...)) {}
5964
};
6065

6166
#endif

‎include/bout/output.hxx‎

Lines changed: 44 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -31,17 +31,17 @@ class Output;
3131

3232
#include "bout/multiostream.hxx"
3333
#include <fstream>
34-
#include <functional>
3534
#include <iostream>
3635
#include <string>
3736

3837
#include "bout/assert.hxx"
39-
#include "bout/boutexception.hxx"
40-
#include "bout/sys/gettext.hxx" // for gettext _() macro
38+
#include "bout/sys/gettext.hxx" // IWYU pragma: keep for gettext _() macro
4139
#include "bout/unused.hxx"
4240

4341
#include "fmt/core.h"
4442

43+
#include <utility>
44+
4545
using std::endl;
4646

4747
/// Class for text output to stdout and/or log file
@@ -61,22 +61,28 @@ using std::endl;
6161
class Output : private multioutbuf_init<char, std::char_traits<char>>,
6262
public std::basic_ostream<char, std::char_traits<char>> {
6363

64-
using _Tr = std::char_traits<char>;
65-
using multioutbuf_init = ::multioutbuf_init<char, _Tr>;
64+
using Tr = std::char_traits<char>;
65+
using multioutbuf_init = ::multioutbuf_init<char, Tr>;
6666

6767
public:
68-
Output() : multioutbuf_init(), std::basic_ostream<char, _Tr>(multioutbuf_init::buf()) {
68+
Output() : multioutbuf_init(), std::basic_ostream<char, Tr>(multioutbuf_init::buf()) {
6969
Output::enable();
7070
}
7171

72+
Output(const Output&) = delete;
73+
Output(Output&&) = delete;
74+
Output& operator=(const Output&) = delete;
75+
Output& operator=(Output&&) = delete;
76+
7277
/// Specify a log file to open
7378
Output(const std::string& filename) {
7479
Output::enable();
7580
open(filename);
7681
}
7782

7883
template <class S, class... Args>
79-
Output(const S& format, const Args&... args) : Output(fmt::format(format, args...)) {}
84+
Output(const S& format, Args&&... args)
85+
: Output(fmt::format(format, std::forward<decltype(args)>(args)...)) {}
8086

8187
~Output() override { close(); }
8288

@@ -87,8 +93,8 @@ public:
8793
int open(const std::string& filename);
8894

8995
template <class S, class... Args>
90-
int open(const S& format, const Args&... args) {
91-
return open(fmt::format(format, args...));
96+
int open(const S& format, Args&&... args) {
97+
return open(fmt::format(format, std::forward<decltype(args)>(args)...));
9298
}
9399

94100
/// Close the log file
@@ -98,24 +104,22 @@ public:
98104
virtual void write(const std::string& message);
99105

100106
template <class S, class... Args>
101-
void write(const S& format, const Args&... args) {
102-
write(fmt::format(format, args...));
107+
void write(const S& format, Args&&... args) {
108+
write(fmt::format(format, std::forward<decltype(args)>(args)...));
103109
}
104110
/// Same as write, but only to screen
105111
virtual void print(const std::string& message);
106112

107113
template <class S, class... Args>
108-
void print(const S& format, const Args&... args) {
109-
print(fmt::format(format, args...));
114+
void print(const S& format, Args&&... args) {
115+
print(fmt::format(format, std::forward<decltype(args)>(args)...));
110116
}
111117

112118
/// Add an output stream. All output will be sent to all streams
113-
void add(std::basic_ostream<char, _Tr>& str) { multioutbuf_init::buf()->add(str); }
119+
void add(std::basic_ostream<char, Tr>& str) { multioutbuf_init::buf()->add(str); }
114120

115121
/// Remove an output stream
116-
void remove(std::basic_ostream<char, _Tr>& str) {
117-
multioutbuf_init::buf()->remove(str);
118-
}
122+
void remove(std::basic_ostream<char, Tr>& str) { multioutbuf_init::buf()->remove(str); }
119123

120124
static Output* getInstance(); ///< Return pointer to instance
121125

@@ -126,7 +130,7 @@ protected:
126130

127131
private:
128132
std::ofstream file; ///< Log file stream
129-
bool enabled; ///< Whether output to stdout is enabled
133+
bool enabled{}; ///< Whether output to stdout is enabled
130134
};
131135

132136
/// Class which behaves like Output, but has no effect.
@@ -136,14 +140,14 @@ private:
136140
class DummyOutput : public Output {
137141
public:
138142
template <class S, class... Args>
139-
void write([[maybe_unused]] const S& format, [[maybe_unused]] const Args&... args){};
143+
void write([[maybe_unused]] const S& format, [[maybe_unused]] Args&&... args) {};
140144
template <class S, class... Args>
141-
void print([[maybe_unused]] const S& format, [[maybe_unused]] const Args&... args){};
142-
void write([[maybe_unused]] const std::string& message) override{};
143-
void print([[maybe_unused]] const std::string& message) override{};
144-
void enable() override{};
145-
void disable() override{};
146-
void enable([[maybe_unused]] bool enable){};
145+
void print([[maybe_unused]] const S& format, [[maybe_unused]] Args&&... args) {};
146+
void write([[maybe_unused]] const std::string& message) override {};
147+
void print([[maybe_unused]] const std::string& message) override {};
148+
void enable() override {};
149+
void disable() override {};
150+
void enable([[maybe_unused]] bool enable) {};
147151
bool isEnabled() override { return false; }
148152
};
149153

@@ -170,10 +174,10 @@ public:
170174
void write(const std::string& message) override;
171175

172176
template <class S, class... Args>
173-
void write(const S& format, const Args&... args) {
177+
void write(const S& format, Args&&... args) {
174178
if (enabled) {
175179
ASSERT1(base != nullptr);
176-
base->write(fmt::format(format, args...));
180+
base->write(fmt::format(format, std::forward<decltype(args)>(args)...));
177181
}
178182
}
179183

@@ -182,10 +186,10 @@ public:
182186
void print(const std::string& message) override;
183187

184188
template <class S, class... Args>
185-
void print(const S& format, const Args&... args) {
189+
void print(const S& format, Args&&... args) {
186190
if (enabled) {
187191
ASSERT1(base != nullptr);
188-
base->print(fmt::format(format, args...));
192+
base->print(fmt::format(format, std::forward<decltype(args)>(args)...));
189193
}
190194
}
191195

@@ -216,8 +220,6 @@ private:
216220
/// The lower-level Output to send output to
217221
Output* base;
218222

219-
protected:
220-
friend class WithQuietOutput;
221223
/// Does this instance output anything?
222224
bool enabled;
223225
};
@@ -276,18 +278,23 @@ ConditionalOutput& operator<<(ConditionalOutput& out, const T* t) {
276278
/// // output now enabled
277279
class WithQuietOutput {
278280
public:
279-
explicit WithQuietOutput(ConditionalOutput& output_in) : output(output_in) {
280-
state = output.enabled;
281-
output.disable();
281+
WithQuietOutput(const WithQuietOutput&) = delete;
282+
WithQuietOutput(WithQuietOutput&&) = delete;
283+
WithQuietOutput& operator=(const WithQuietOutput&) = delete;
284+
WithQuietOutput& operator=(WithQuietOutput&&) = delete;
285+
explicit WithQuietOutput(ConditionalOutput& output_in)
286+
: output(&output_in), state(output->isEnabled()) {
287+
output->disable();
282288
}
283289

284-
~WithQuietOutput() { output.enable(state); }
290+
~WithQuietOutput() { output->enable(state); }
285291

286292
private:
287-
ConditionalOutput& output;
293+
ConditionalOutput* output;
288294
bool state;
289295
};
290296

297+
// NOLINTBEGIN(cppcoreguidelines-avoid-non-const-global-variables)
291298
/// To allow statements like "output.write(...)" or "output << ..."
292299
/// Output for debugging
293300
#ifdef BOUT_USE_OUTPUT_DEBUG
@@ -303,5 +310,6 @@ extern ConditionalOutput output_verbose; ///< less interesting messages
303310

304311
/// Generic output, given the same level as output_progress
305312
extern ConditionalOutput output;
313+
// NOLINTEND(cppcoreguidelines-avoid-non-const-global-variables)
306314

307315
#endif // BOUT_OUTPUT_H

‎src/sys/boutexception.cxx‎

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,19 @@
44
#include <bout/boutexception.hxx>
55
#include <bout/msg_stack.hxx>
66
#include <bout/utils.hxx>
7+
78
#include <mpi.h>
89

910
#if BOUT_USE_BACKTRACE
1011
#include <dlfcn.h>
1112
#include <execinfo.h>
13+
#include <stdio.h>
1214
#endif
1315

1416
#include <array>
1517
#include <cstdlib>
1618
#include <string>
19+
#include <utility>
1720

1821
#include <fmt/format.h>
1922

@@ -22,18 +25,24 @@ const std::string header{"====== Exception thrown ======\n"};
2225
}
2326

2427
void BoutParallelThrowRhsFail(int status, const char* message) {
25-
int allstatus;
28+
int allstatus = 0;
2629
MPI_Allreduce(&status, &allstatus, 1, MPI_INT, MPI_LOR, BoutComm::get());
2730

28-
if (allstatus) {
31+
if (allstatus != 0) {
2932
throw BoutRhsFail(message);
3033
}
3134
}
3235

33-
BoutException::BoutException(std::string msg) : message(std::move(msg)) {
34-
makeBacktrace();
36+
BoutException::BoutException(std::string msg)
37+
: message(std::move(msg))
38+
#if BOUT_USE_BACKTRACE
39+
,
40+
trace_size(backtrace(trace.data(), TRACE_MAX)),
41+
messages(backtrace_symbols(trace.data(), trace_size))
42+
#endif
43+
{
3544
if (std::getenv("BOUT_SHOW_BACKTRACE") != nullptr) {
36-
message = getBacktrace() + "\n" + message;
45+
message = fmt::format("{}\n{}", getBacktrace(), message);
3746
}
3847
}
3948

@@ -54,7 +63,8 @@ std::string BoutException::getBacktrace() const {
5463
backtrace_message = "====== Exception path ======\n";
5564
// skip first stack frame (points here)
5665
for (int i = trace_size - 1; i > 1; --i) {
57-
backtrace_message += fmt::format(FMT_STRING("[bt] #{:d} {:s}\n"), i - 1, messages[i]);
66+
backtrace_message = fmt::format(FMT_STRING("{}[bt] #{:d} {:s}\n"), backtrace_message,
67+
i - 1, messages[i]);
5868
// find first occurence of '(' or ' ' in message[i] and assume
5969
// everything before that is the file name. (Don't go beyond 0 though
6070
// (string terminator)
@@ -65,11 +75,11 @@ std::string BoutException::getBacktrace() const {
6575

6676
// If we are compiled as PIE, need to get base pointer of .so and substract
6777
Dl_info info;
68-
void* ptr = trace[i];
69-
if (dladdr(trace[i], &info)) {
78+
const void* ptr = trace[i];
79+
if (dladdr(ptr, &info) != 0) {
7080
// Additionally, check whether this is the default offset for an executable
7181
if (info.dli_fbase != reinterpret_cast<void*>(0x400000)) {
72-
ptr = reinterpret_cast<void*>(reinterpret_cast<size_t>(trace[i])
82+
ptr = reinterpret_cast<void*>(reinterpret_cast<size_t>(ptr)
7383
- reinterpret_cast<size_t>(info.dli_fbase));
7484
}
7585
}
@@ -82,27 +92,21 @@ std::string BoutException::getBacktrace() const {
8292
FILE* file = popen(syscom.c_str(), "r");
8393
if (file != nullptr) {
8494
std::array<char, 1024> out{};
85-
char* retstr = nullptr;
95+
const char* retstr = nullptr;
8696
std::string buf;
8797
while ((retstr = fgets(out.data(), out.size() - 1, file)) != nullptr) {
8898
buf += retstr;
8999
}
90100
int const status = pclose(file);
91101
if (status == 0) {
92-
backtrace_message += buf;
102+
backtrace_message = fmt::format("{}{}", backtrace_message, buf);
93103
}
94104
}
95105
}
96106
#else
97107
backtrace_message = "Stacktrace not enabled.\n";
98108
#endif
99109

100-
return backtrace_message + msg_stack.getDump() + "\n" + header + message + "\n";
101-
}
102-
103-
void BoutException::makeBacktrace() {
104-
#if BOUT_USE_BACKTRACE
105-
trace_size = backtrace(trace.data(), TRACE_MAX);
106-
messages = backtrace_symbols(trace.data(), trace_size);
107-
#endif
110+
return fmt::format("{}{}\n{}{}\n", backtrace_message, msg_stack.getDump(), header,
111+
message);
108112
}

‎src/sys/output.cxx‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,13 @@
2323
*
2424
**************************************************************************/
2525

26+
#include <bout/assert.hxx>
2627
#include <bout/output.hxx>
27-
#include <bout/utils.hxx>
28-
#include <cstdarg>
29-
#include <cstdio>
30-
#include <cstring>
28+
29+
#include <fmt/base.h>
30+
31+
#include <iostream>
32+
#include <string>
3133

3234
void Output::enable() {
3335
add(std::cout);
@@ -68,7 +70,7 @@ void Output::close() {
6870
}
6971

7072
void Output::write(const std::string& message) {
71-
multioutbuf_init::buf()->sputn(message.c_str(), message.length());
73+
multioutbuf_init::buf()->sputn(message.c_str(), static_cast<long>(message.length()));
7274
}
7375

7476
void Output::print(const std::string& message) {
@@ -98,6 +100,7 @@ void ConditionalOutput::print(const std::string& message) {
98100
}
99101
}
100102

103+
// NOLINTBEGIN(cppcoreguidelines-avoid-non-const-global-variables)
101104
#ifdef BOUT_USE_OUTPUT_DEBUG
102105
ConditionalOutput output_debug(Output::getInstance());
103106
#else
@@ -109,3 +112,4 @@ ConditionalOutput output_progress(Output::getInstance());
109112
ConditionalOutput output_error(Output::getInstance());
110113
ConditionalOutput output_verbose(Output::getInstance(), false);
111114
ConditionalOutput output(Output::getInstance());
115+
// NOLINTEND(cppcoreguidelines-avoid-non-const-global-variables)

0 commit comments

Comments
 (0)