Skip to content

Commit 6c33fbc

Browse files
committed
CommandLineParser: Replace global sout/serr streams with class members
- This removes the global variable and prevents stderr/stdout from being printed in tests
1 parent ce11ebb commit 6c33fbc

File tree

4 files changed

+58
-40
lines changed

4 files changed

+58
-40
lines changed

solc/CommandLineInterface.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -497,11 +497,11 @@ void CommandLineInterface::createJson(string const& _fileName, string const& _js
497497

498498
bool CommandLineInterface::parseArguments(int _argc, char const* const* _argv)
499499
{
500-
CommandLineParser parser;
500+
CommandLineParser parser(sout(false), serr(false));
501501
bool success = parser.parse(_argc, _argv, isatty(fileno(stdin)));
502502
if (!success)
503503
return false;
504-
g_hasOutput = g_hasOutput || CommandLineParser::hasOutput();
504+
g_hasOutput = g_hasOutput || parser.hasOutput();
505505
m_options = parser.options();
506506

507507
return true;

solc/CommandLineParser.cpp

Lines changed: 11 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -34,24 +34,16 @@ namespace po = boost::program_options;
3434
namespace solidity::frontend
3535
{
3636

37-
static bool g_hasOutput = false;
38-
39-
namespace
40-
{
41-
42-
std::ostream& sout()
37+
ostream& CommandLineParser::sout()
4338
{
44-
g_hasOutput = true;
45-
return cout;
39+
m_hasOutput = true;
40+
return m_sout;
4641
}
4742

48-
std::ostream& serr(bool _used = true)
43+
ostream& CommandLineParser::serr()
4944
{
50-
if (_used)
51-
g_hasOutput = true;
52-
return cerr;
53-
}
54-
45+
m_hasOutput = true;
46+
return m_serr;
5547
}
5648

5749
#define cout
@@ -233,8 +225,7 @@ static set<string> const g_metadataHashArgs
233225
g_strNone
234226
};
235227

236-
[[noreturn]]
237-
static void printVersionAndExit()
228+
void CommandLineParser::printVersionAndExit()
238229
{
239230
sout() <<
240231
"solc, the solidity compiler commandline interface" <<
@@ -245,19 +236,16 @@ static void printVersionAndExit()
245236
exit(EXIT_SUCCESS);
246237
}
247238

248-
[[noreturn]]
249-
static void printLicenseAndExit()
239+
void CommandLineParser::printLicenseAndExit()
250240
{
251241
sout() << otherLicenses << endl;
252242
// This is a static variable generated by cmake from LICENSE.txt
253243
sout() << licenseText << endl;
254244
exit(EXIT_SUCCESS);
255245
}
256246

257-
namespace
258-
{
259247

260-
bool checkMutuallyExclusive(boost::program_options::variables_map const& args, std::string const& _optionA, std::string const& _optionB)
248+
bool CommandLineParser::checkMutuallyExclusive(boost::program_options::variables_map const& args, string const& _optionA, string const& _optionB)
261249
{
262250
if (args.count(_optionA) && args.count(_optionB))
263251
{
@@ -268,8 +256,6 @@ bool checkMutuallyExclusive(boost::program_options::variables_map const& args, s
268256
return true;
269257
}
270258

271-
}
272-
273259
bool CompilerOutputs::operator==(CompilerOutputs const& _other) const noexcept
274260
{
275261
static_assert(
@@ -489,7 +475,7 @@ bool CommandLineParser::parseLibraryOption(string const& _input)
489475

490476
bool CommandLineParser::parse(int _argc, char const* const* _argv, bool interactiveTerminal)
491477
{
492-
g_hasOutput = false;
478+
m_hasOutput = false;
493479

494480
// Declare the supported options.
495481
po::options_description desc((R"(solc, the Solidity commandline compiler.
@@ -1186,11 +1172,6 @@ General Information)").c_str(),
11861172
return true;
11871173
}
11881174

1189-
bool CommandLineParser::hasOutput()
1190-
{
1191-
return g_hasOutput;
1192-
}
1193-
11941175
bool CommandLineParser::parseCombinedJsonOption()
11951176
{
11961177
if (!m_args.count(g_argCombinedJson))
@@ -1242,4 +1223,5 @@ string CommandLineParser::joinOptionNames(vector<string> const& _optionNames, st
12421223
_separator
12431224
);
12441225
}
1226+
12451227
} // namespace solidity::frontend

solc/CommandLineParser.h

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include <map>
3434
#include <memory>
3535
#include <optional>
36+
#include <ostream>
3637
#include <set>
3738
#include <string>
3839
#include <vector>
@@ -177,6 +178,11 @@ struct CommandLineOptions
177178
class CommandLineParser
178179
{
179180
public:
181+
explicit CommandLineParser(std::ostream& _sout, std::ostream& _serr):
182+
m_sout(_sout),
183+
m_serr(_serr)
184+
{}
185+
180186
/// Parses the command-line arguments and fills out the internal CommandLineOptions structure.
181187
/// Performs validation and prints error messages. If requested, prints usage banner, version
182188
/// or license.
@@ -191,9 +197,8 @@ class CommandLineParser
191197

192198
CommandLineOptions const& options() const { return m_options; }
193199

194-
/// Returns true if any parser instance has written anything to cout or cerr since the last
195-
/// call to parse().
196-
static bool hasOutput();
200+
/// Returns true if the parser has written anything to any of its output streams.
201+
bool hasOutput() const { return m_hasOutput; }
197202

198203
private:
199204
/// Parses the value supplied to --combined-json.
@@ -210,9 +215,28 @@ class CommandLineParser
210215
/// @return false if there are any validation errors, true otherwise.
211216
bool parseLibraryOption(std::string const& _input);
212217

218+
bool checkMutuallyExclusive(
219+
boost::program_options::variables_map const& args,
220+
std::string const& _optionA,
221+
std::string const& _optionB
222+
);
223+
[[noreturn]] void printVersionAndExit();
224+
[[noreturn]] void printLicenseAndExit();
213225
size_t countEnabledOptions(std::vector<std::string> const& _optionNames) const;
214226
static std::string joinOptionNames(std::vector<std::string> const& _optionNames, std::string _separator = ", ");
215227

228+
/// Returns the stream that should receive normal output. Sets m_hasOutput to true if the
229+
/// stream has ever been used.
230+
std::ostream& sout();
231+
232+
/// Returns the stream that should receive error output. Sets m_hasOutput to true if the
233+
/// stream has ever been used.
234+
std::ostream& serr();
235+
236+
std::ostream& m_sout;
237+
std::ostream& m_serr;
238+
bool m_hasOutput = false;
239+
216240
CommandLineOptions m_options;
217241

218242
/// Map of command-line arguments produced by boost::program_options.

test/solc/CommandLineParser.cpp

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ using namespace solidity::yul;
4646
namespace
4747
{
4848

49-
optional<CommandLineOptions> parseCommandLine(vector<string> const& commandLine)
49+
optional<CommandLineOptions> parseCommandLine(vector<string> const& commandLine, ostream& _stdout, ostream& _stderr)
5050
{
5151
size_t argc = commandLine.size();
5252
vector<char const*> argv(argc + 1);
@@ -57,7 +57,7 @@ optional<CommandLineOptions> parseCommandLine(vector<string> const& commandLine)
5757
for (size_t i = 0; i < argc; ++i)
5858
argv[i] = commandLine[i].c_str();
5959

60-
CommandLineParser cliParser;
60+
CommandLineParser cliParser(_stdout, _stderr);
6161
bool success = cliParser.parse(
6262
static_cast<int>(argc),
6363
argv.data(),
@@ -93,8 +93,11 @@ BOOST_AUTO_TEST_CASE(no_options)
9393
nullopt,
9494
};
9595

96-
optional<CommandLineOptions> parsedOptions = parseCommandLine(commandLine);
96+
stringstream sout, serr;
97+
optional<CommandLineOptions> parsedOptions = parseCommandLine(commandLine, sout, serr);
9798

99+
BOOST_TEST(sout.str() == "");
100+
BOOST_TEST(serr.str() == "");
98101
BOOST_REQUIRE(parsedOptions.has_value());
99102
BOOST_TEST((parsedOptions.value() == expectedOptions));
100103
}
@@ -202,8 +205,11 @@ BOOST_AUTO_TEST_CASE(cli_mode_options)
202205
5,
203206
};
204207

205-
optional<CommandLineOptions> parsedOptions = parseCommandLine(commandLine);
208+
stringstream sout, serr;
209+
optional<CommandLineOptions> parsedOptions = parseCommandLine(commandLine, sout, serr);
206210

211+
BOOST_TEST(sout.str() == "");
212+
BOOST_TEST(serr.str() == "");
207213
BOOST_REQUIRE(parsedOptions.has_value());
208214
BOOST_TEST((parsedOptions.value() == expectedOptions));
209215
}
@@ -316,8 +322,11 @@ BOOST_AUTO_TEST_CASE(assembly_mode_options)
316322
expectedOptions.optimizer.yulSteps = "agf";
317323
}
318324

319-
optional<CommandLineOptions> parsedOptions = parseCommandLine(commandLine);
325+
stringstream sout, serr;
326+
optional<CommandLineOptions> parsedOptions = parseCommandLine(commandLine, sout, serr);
320327

328+
BOOST_TEST(sout.str() == "");
329+
BOOST_TEST(serr.str() == "Warning: Yul is still experimental. Please use the output with care.\n");
321330
BOOST_REQUIRE(parsedOptions.has_value());
322331

323332
BOOST_TEST((parsedOptions.value() == expectedOptions));
@@ -388,8 +397,11 @@ BOOST_AUTO_TEST_CASE(standard_json_mode_options)
388397
expectedOptions.compiler.combinedJsonRequests->abi = true;
389398
expectedOptions.compiler.combinedJsonRequests->binary = true;
390399

391-
optional<CommandLineOptions> parsedOptions = parseCommandLine(commandLine);
400+
stringstream sout, serr;
401+
optional<CommandLineOptions> parsedOptions = parseCommandLine(commandLine, sout, serr);
392402

403+
BOOST_TEST(sout.str() == "");
404+
BOOST_TEST(serr.str() == "");
393405
BOOST_REQUIRE(parsedOptions.has_value());
394406
BOOST_TEST((parsedOptions.value() == expectedOptions));
395407
}

0 commit comments

Comments
 (0)