Skip to content

Commit

Permalink
CommandLineParser: Replace global sout/serr streams with class members
Browse files Browse the repository at this point in the history
- This removes the global variable and prevents stderr/stdout from being printed in tests
  • Loading branch information
cameel committed Jun 10, 2021
1 parent fcab7e0 commit 4b87f33
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 38 deletions.
4 changes: 2 additions & 2 deletions solc/CommandLineInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -474,11 +474,11 @@ void CommandLineInterface::createJson(string const& _fileName, string const& _js

bool CommandLineInterface::parseArguments(int _argc, char** _argv)
{
CommandLineParser parser;
CommandLineParser parser(sout(false), serr(false));
bool success = parser.parse(_argc, _argv);
if (!success)
return false;
g_hasOutput = g_hasOutput || CommandLineParser::hasOutput();
g_hasOutput = g_hasOutput || parser.hasOutput();
m_options = parser.options();

return true;
Expand Down
38 changes: 11 additions & 27 deletions solc/CommandLineParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,24 +49,16 @@ namespace po = boost::program_options;
namespace solidity::frontend
{

static bool g_hasOutput = false;

namespace
{

std::ostream& sout()
ostream& CommandLineParser::sout()
{
g_hasOutput = true;
return cout;
m_hasOutput = true;
return m_sout;
}

std::ostream& serr(bool _used = true)
ostream& CommandLineParser::serr()
{
if (_used)
g_hasOutput = true;
return cerr;
}

m_hasOutput = true;
return m_serr;
}

#define cout
Expand Down Expand Up @@ -248,7 +240,7 @@ static set<string> const g_metadataHashArgs
g_strNone
};

static void version()
void CommandLineParser::version()
{
sout() <<
"solc, the solidity compiler commandline interface" <<
Expand All @@ -259,18 +251,16 @@ static void version()
exit(0);
}

static void license()
void CommandLineParser::license()
{
sout() << otherLicenses << endl;
// This is a static variable generated by cmake from LICENSE.txt
sout() << licenseText << endl;
exit(0);
}

namespace
{

bool checkMutuallyExclusive(boost::program_options::variables_map const& args, std::string const& _optionA, std::string const& _optionB)
bool CommandLineParser::checkMutuallyExclusive(boost::program_options::variables_map const& args, string const& _optionA, string const& _optionB)
{
if (args.count(_optionA) && args.count(_optionB))
{
Expand All @@ -281,8 +271,6 @@ bool checkMutuallyExclusive(boost::program_options::variables_map const& args, s
return true;
}

}

bool OutputSelection::operator==(OutputSelection const& _other) const
{
static_assert(
Expand Down Expand Up @@ -498,7 +486,7 @@ bool CommandLineParser::parseLibraryOption(string const& _input)

bool CommandLineParser::parse(int _argc, char const* const* _argv)
{
g_hasOutput = false;
m_hasOutput = false;

// Declare the supported options.
po::options_description desc((R"(solc, the Solidity commandline compiler.
Expand Down Expand Up @@ -1194,11 +1182,6 @@ General Information)").c_str(),
return true;
}

bool CommandLineParser::hasOutput()
{
return g_hasOutput;
}

bool CommandLineParser::parseCombinedJsonOption()
{
if (!m_args.count(g_argCombinedJson))
Expand Down Expand Up @@ -1250,4 +1233,5 @@ string CommandLineParser::joinOptionNames(vector<string> const& _optionNames, st
_separator
);
}

} // namespace solidity::frontend
29 changes: 26 additions & 3 deletions solc/CommandLineParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,11 @@ struct CommandLineOptions
class CommandLineParser
{
public:
explicit CommandLineParser(std::ostream& _sout, std::ostream& _serr):
m_sout(_sout),
m_serr(_serr)
{}

/// Parses the command-line arguments and fills out the internal CommandLineOptions structure.
/// Performs validation and prints error messages. If requested, prints usage banner, version
/// or license.
Expand All @@ -172,9 +177,8 @@ class CommandLineParser

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

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

private:
/// Parses the value supplied to --combined-json.
Expand All @@ -191,9 +195,28 @@ class CommandLineParser
/// @return false if there are any validation errors, true otherwise.
bool parseLibraryOption(std::string const& _input);

bool checkMutuallyExclusive(
boost::program_options::variables_map const& args,
std::string const& _optionA,
std::string const& _optionB
);
void version();
void license();
size_t countEnabledOptions(std::vector<std::string> const& _optionNames) const;
static std::string joinOptionNames(std::vector<std::string> const& _optionNames, std::string _separator = ", ");

/// Returns the stream that should receive normal output. Sets m_hasOutput to true if the
/// stream has ever been used.
std::ostream& sout();

/// Returns the stream that should receive error output. Sets m_hasOutput to true if the
/// stream has ever been used.
std::ostream& serr();

bool m_hasOutput = false;
std::ostream& m_sout;
std::ostream& m_serr;

CommandLineOptions m_options;

/// Map of command-line arguments produced by boost::program_options.
Expand Down
24 changes: 18 additions & 6 deletions test/solc/CommandLineParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ using namespace solidity::yul;
namespace
{

optional<CommandLineOptions> parseCommandLine(vector<string> const& commandLine)
optional<CommandLineOptions> parseCommandLine(vector<string> const& commandLine, ostream& _stdout, ostream& _stderr)
{
size_t argc = commandLine.size();
vector<char const*> argv(commandLine.size() + 1);
Expand All @@ -55,7 +55,7 @@ optional<CommandLineOptions> parseCommandLine(vector<string> const& commandLine)
for (size_t i = 0; i < argc; ++i)
argv[i] = commandLine[i].c_str();

CommandLineParser cliParser;
CommandLineParser cliParser(_stdout, _stderr);
bool success = cliParser.parse(static_cast<int>(argc), argv.data());

if (!success)
Expand Down Expand Up @@ -88,8 +88,11 @@ BOOST_AUTO_TEST_CASE(no_options)
nullopt,
};

optional<CommandLineOptions> parsedOptions = parseCommandLine(commandLine);
stringstream sout, serr;
optional<CommandLineOptions> parsedOptions = parseCommandLine(commandLine, sout, serr);

BOOST_TEST(sout.str() == "");
BOOST_TEST(serr.str() == "");
BOOST_REQUIRE(parsedOptions.has_value());
BOOST_TEST((parsedOptions.value() == expectedOptions));
}
Expand Down Expand Up @@ -200,8 +203,11 @@ BOOST_AUTO_TEST_CASE(cli_mode_options)
5,
};

optional<CommandLineOptions> parsedOptions = parseCommandLine(commandLine);
stringstream sout, serr;
optional<CommandLineOptions> parsedOptions = parseCommandLine(commandLine, sout, serr);

BOOST_TEST(sout.str() == "");
BOOST_TEST(serr.str() == "");
BOOST_REQUIRE(parsedOptions.has_value());
BOOST_TEST((parsedOptions.value() == expectedOptions));
}
Expand Down Expand Up @@ -314,8 +320,11 @@ BOOST_AUTO_TEST_CASE(assembly_mode_options)
expectedOptions.yulOptimiserSteps = "agf";
}

optional<CommandLineOptions> parsedOptions = parseCommandLine(commandLine);
stringstream sout, serr;
optional<CommandLineOptions> parsedOptions = parseCommandLine(commandLine, sout, serr);

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

BOOST_TEST((parsedOptions.value() == expectedOptions));
Expand Down Expand Up @@ -386,8 +395,11 @@ BOOST_AUTO_TEST_CASE(standard_json_mode_options)
expectedOptions.combinedJsonRequests->abi = true;
expectedOptions.combinedJsonRequests->binary = true;

optional<CommandLineOptions> parsedOptions = parseCommandLine(commandLine);
stringstream sout, serr;
optional<CommandLineOptions> parsedOptions = parseCommandLine(commandLine, sout, serr);

BOOST_TEST(sout.str() == "");
BOOST_TEST(serr.str() == "");
BOOST_REQUIRE(parsedOptions.has_value());
BOOST_TEST((parsedOptions.value() == expectedOptions));
}
Expand Down

0 comments on commit 4b87f33

Please sign in to comment.