Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions solc/CommandLineParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,23 @@ void CommandLineParser::checkMutuallyExclusive(std::vector<std::string> const& _
}
}

std::vector<std::string> const& CommandLineParser::experimentalOptionNames()
{
static std::vector<std::string> const names{
g_strLSP,
g_strImportAst,
g_strImportEvmAssemblerJson,
"ir-ast-json",
"ir-optimized-ast-json",
"yul-cfg-json",
"ethdebug",
"ethdebug-runtime",
g_strEOFVersion,
g_strViaSSACFG,
};
return names;
}

void CommandLineParser::checkExperimental(std::vector<std::string> const& _optionNames) const
{
if (!m_args.contains(g_strExperimental) && countEnabledOptions(_optionNames) > 0)
Expand Down Expand Up @@ -994,18 +1011,7 @@ void CommandLineParser::processArgs()
g_strImportEvmAssemblerJson,
});

checkExperimental({
g_strLSP,
g_strImportAst,
g_strImportEvmAssemblerJson,
"ir-ast-json",
"ir-optimized-ast-json",
"yul-cfg-json",
"ethdebug",
"ethdebug-runtime",
g_strEOFVersion,
g_strViaSSACFG,
});
checkExperimental(experimentalOptionNames());

if (m_args.count(g_strHelp) > 0)
m_options.input.mode = InputMode::Help;
Expand Down
3 changes: 3 additions & 0 deletions solc/CommandLineParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,9 @@ class CommandLineParser

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

/// yields CLI option names (without leading "--") that require --experimental.
static std::vector<std::string> const& experimentalOptionNames();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i was itching to make this a static std::array constexpr but that would've been a bigger refactor and entailed making all the global static strings in the implementation file g_str* string views or similar and update the various string concatenations in the cli to fmt::format style. didn't seem worth the effort in the end.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean you'd replace them with global static string_views? Don't they still need to be attached to some string stored somewhere? What would that look like?

TBH I have mixed feelings about those global strings. I kept them back when I refactored the CLI, but they're clunky, not used consistently for all options and I'm not sure this really saves us all that much. The biggest benefit is I guess preventing typos when you have to use an existing option name in a bunch more places, but that's not much.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean you'd replace them with global static string_views? Don't they still need to be attached to some string stored somewhere? What would that look like?

If you write static std::string_view constexpr v{"hi"} then that string literal in there already has static storage duration and so the string view can just point to that - they have constexpr support :)

So something like this works, too:

static auto constexpr opts = std::to_array<std::string_view>({
    "A", "B", "C"
});

TBH I have mixed feelings about those global strings.

Not the biggest fan of them, either. I agree, they have dubious value here... I'd be much happier with a struct that defines the properties of an option somewhere and having a couple of these.


static void printHelp(std::ostream& _out) { _out << optionsDescription(); }

private:
Expand Down
46 changes: 25 additions & 21 deletions test/solc/CommandLineParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
#include <libsmtutil/SolverInterface.h>
#include <libsolidity/interface/Version.h>

#include <range/v3/algorithm/find.hpp>

#include <map>
#include <optional>
#include <ostream>
Expand Down Expand Up @@ -429,20 +431,20 @@ BOOST_AUTO_TEST_CASE(invalid_options_input_modes_combinations)
{
std::map<std::string, std::vector<std::string>> invalidOptionInputModeCombinations = {
// TODO: This should eventually contain all options.
{"--experimental-via-ir", {"--assemble", "--strict-assembly", "--standard-json", "--link"}},
{"--via-ir", {"--assemble", "--strict-assembly", "--standard-json", "--link"}},
{"--metadata-literal", {"--assemble", "--strict-assembly", "--standard-json", "--link"}},
{"--metadata-hash=swarm", {"--assemble", "--strict-assembly", "--standard-json", "--link"}},
{"--model-checker-show-proved-safe", {"--assemble", "--strict-assembly", "--standard-json", "--link"}},
{"--model-checker-show-unproved", {"--assemble", "--strict-assembly", "--standard-json", "--link"}},
{"--model-checker-show-unsupported", {"--assemble", "--strict-assembly", "--standard-json", "--link"}},
{"--model-checker-div-mod-no-slacks", {"--assemble", "--strict-assembly", "--standard-json", "--link"}},
{"--model-checker-engine=bmc", {"--assemble", "--strict-assembly", "--standard-json", "--link"}},
{"--model-checker-invariants=contract,reentrancy", {"--assemble", "--strict-assembly", "--standard-json", "--link"}},
{"--model-checker-solvers=z3,smtlib2", {"--assemble", "--strict-assembly", "--standard-json", "--link"}},
{"--model-checker-timeout=5", {"--assemble", "--strict-assembly", "--standard-json", "--link"}},
{"--model-checker-contracts=contract1.yul:A,contract2.yul:B", {"--assemble", "--strict-assembly", "--standard-json", "--link"}},
{"--model-checker-targets=underflow,divByZero", {"--assemble", "--strict-assembly", "--standard-json", "--link"}},
{"--experimental-via-ir", {"--assemble", "--strict-assembly", "--standard-json", "--link", "--import-asm-json"}},
{"--via-ir", {"--assemble", "--strict-assembly", "--standard-json", "--link", "--import-asm-json"}},
{"--metadata-literal", {"--assemble", "--strict-assembly", "--standard-json", "--link", "--import-asm-json"}},
{"--metadata-hash=swarm", {"--assemble", "--strict-assembly", "--standard-json", "--link", "--import-asm-json"}},
{"--model-checker-show-proved-safe", {"--assemble", "--strict-assembly", "--standard-json", "--link", "--import-asm-json"}},
{"--model-checker-show-unproved", {"--assemble", "--strict-assembly", "--standard-json", "--link", "--import-asm-json"}},
{"--model-checker-show-unsupported", {"--assemble", "--strict-assembly", "--standard-json", "--link", "--import-asm-json"}},
{"--model-checker-div-mod-no-slacks", {"--assemble", "--strict-assembly", "--standard-json", "--link", "--import-asm-json"}},
{"--model-checker-engine=bmc", {"--assemble", "--strict-assembly", "--standard-json", "--link", "--import-asm-json"}},
{"--model-checker-invariants=contract,reentrancy", {"--assemble", "--strict-assembly", "--standard-json", "--link", "--import-asm-json"}},
{"--model-checker-solvers=z3,smtlib2", {"--assemble", "--strict-assembly", "--standard-json", "--link", "--import-asm-json"}},
{"--model-checker-timeout=5", {"--assemble", "--strict-assembly", "--standard-json", "--link", "--import-asm-json"}},
{"--model-checker-contracts=contract1.yul:A,contract2.yul:B", {"--assemble", "--strict-assembly", "--standard-json", "--link", "--import-asm-json"}},
{"--model-checker-targets=underflow,divByZero", {"--assemble", "--strict-assembly", "--standard-json", "--link", "--import-asm-json"}},
{"--via-ssa-cfg", {"--standard-json", "--link", "--import-asm-json"}}
};

Expand All @@ -455,12 +457,14 @@ BOOST_AUTO_TEST_CASE(invalid_options_input_modes_combinations)
soltestAssert(!optionNameWithoutValue.empty());

std::vector<std::string> commandLine = {"solc", optionName, "file", inputMode};
bool experimentalMode = false;
if (optionNameWithoutValue == "--via-ssa-cfg")
{
commandLine.push_back("--experimental");
experimentalMode = true;
}

static auto constexpr isExperimental = [](std::string_view const _cliFlag) -> bool {
auto const& experimentalOpts = CommandLineParser::experimentalOptionNames();
return ranges::find(experimentalOpts, _cliFlag.substr(2)) != ranges::end(experimentalOpts);
};
bool needsExperimentalMode = isExperimental(optionNameWithoutValue) || isExperimental(inputMode);
if (needsExperimentalMode)
commandLine.emplace_back("--experimental");

std::string expectedMessage = "The following options are not supported in the current input mode: " + optionNameWithoutValue;
// When --experimental is combined with --standard-json, a different error fires first
Expand All @@ -469,7 +473,7 @@ BOOST_AUTO_TEST_CASE(invalid_options_input_modes_combinations)
{
std::string const what = _exception.what();
return what == expectedMessage || (
experimentalMode &&
needsExperimentalMode &&
what.starts_with("Standard JSON input mode is incompatible with the --experimental flag.")
);
};
Expand Down