Skip to content

Commit

Permalink
refactor: Use tl::expected for process_args return value
Browse files Browse the repository at this point in the history 
This should make the error paths more obvious.

As discussed in ccache#1459.
  • Loading branch information
@jrosdahl
jrosdahl committed May 27, 2024
1 parent 5717e48 commit 181ffcf
Show file tree
Hide file tree
Showing 4 changed files with 266 additions and 287 deletions.
48 changes: 25 additions & 23 deletions src/ccache/argprocessing.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1253,7 +1253,7 @@ get_default_pch_file_extension(const Config& config)

} // namespace

ProcessArgsResult
tl::expected<ProcessArgsResult, core::Statistic>
process_args(Context& ctx)
{
ASSERT(!ctx.orig_args.empty());
Expand Down Expand Up @@ -1283,18 +1283,19 @@ process_args(Context& ctx)

if (state.input_files.empty()) {
LOG_RAW("No input file found");
return Statistic::no_input_file;
return tl::unexpected(Statistic::no_input_file);
}
if (state.input_files.size() > 1) {
if (is_link) {
LOG_RAW("Called for link");
return pstr(state.input_files.front()).str().find("conftest.")
!= std::string::npos
? Statistic::autoconf_test
: Statistic::called_for_link;
return tl::unexpected(
pstr(state.input_files.front()).str().find("conftest.")
!= std::string::npos
? Statistic::autoconf_test
: Statistic::called_for_link);
} else {
LOG_RAW("Multiple input files");
return Statistic::multiple_source_files;
return tl::unexpected(Statistic::multiple_source_files);
}
}

Expand All @@ -1310,21 +1311,21 @@ process_args(Context& ctx)
// potentially support this by behaving differently depending on the
// compiler type, but let's just bail out for now.
LOG_RAW("-Wp,-M[M]D in combination with -MF is not supported");
return Statistic::unsupported_compiler_option;
return tl::unexpected(Statistic::unsupported_compiler_option);
}

if (!state.last_seen_msvc_z_debug_option.empty()
&& state.last_seen_msvc_z_debug_option.substr(2) != "7") {
// /Zi and /ZI are unsupported, but /Z7 is fine.
LOG("Compiler option {} is unsupported",
state.last_seen_msvc_z_debug_option);
return Statistic::unsupported_compiler_option;
return tl::unexpected(Statistic::unsupported_compiler_option);
}

// Don't try to second guess the compiler's heuristics for stdout handling.
if (args_info.output_obj == "-") {
LOG_RAW("Output file is -");
return Statistic::output_to_stdout;
return tl::unexpected(Statistic::output_to_stdout);
}

// Determine output object file.
Expand Down Expand Up @@ -1366,7 +1367,7 @@ process_args(Context& ctx)
|| DirEntry(args_info.orig_included_pch_file).is_directory())) {
LOG("Unsupported folder path value for -Fp: {}",
args_info.included_pch_file);
return Statistic::could_not_use_precompiled_header;
return tl::unexpected(Statistic::could_not_use_precompiled_header);
}

if (included_pch_file_by_source && !args_info.input_file.empty()) {
Expand All @@ -1388,7 +1389,7 @@ process_args(Context& ctx)
// args_info.output_obj which is needed to determine the log filename in
// CCACHE_DEBUG mode.
if (argument_error) {
return *argument_error;
return tl::unexpected(*argument_error);
}

if (state.generating_debuginfo_level_3 && !config.run_second_cpp()) {
Expand All @@ -1414,7 +1415,7 @@ process_args(Context& ctx)
"You have to specify \"time_macros\" sloppiness when using"
" precompiled headers to get direct hits");
LOG_RAW("Disabling direct mode");
return Statistic::could_not_use_precompiled_header;
return tl::unexpected(Statistic::could_not_use_precompiled_header);
}
}

Expand All @@ -1434,7 +1435,7 @@ process_args(Context& ctx)
if (!state.explicit_language.empty()) {
if (!language_is_supported(state.explicit_language)) {
LOG("Unsupported language: {}", state.explicit_language);
return Statistic::unsupported_source_language;
return tl::unexpected(Statistic::unsupported_source_language);
}
args_info.actual_language = state.explicit_language;
} else {
Expand All @@ -1458,7 +1459,7 @@ process_args(Context& ctx)
LOG_RAW(
"You have to specify \"pch_defines,time_macros\" sloppiness when"
" creating precompiled headers");
return Statistic::could_not_use_precompiled_header;
return tl::unexpected(Statistic::could_not_use_precompiled_header);
}

if (is_link) {
Expand All @@ -1468,15 +1469,16 @@ process_args(Context& ctx)
LOG_RAW("No -c option found");
// Having a separate statistic for autoconf tests is useful, as they are
// the dominant form of "called for link" in many cases.
return args_info.input_file.find("conftest.") != std::string::npos
? Statistic::autoconf_test
: Statistic::called_for_link;
return tl::unexpected(args_info.input_file.find("conftest.")
!= std::string::npos
? Statistic::autoconf_test
: Statistic::called_for_link);
}
}

if (args_info.actual_language.empty()) {
LOG("Unsupported source extension: {}", args_info.input_file);
return Statistic::unsupported_source_language;
return tl::unexpected(Statistic::unsupported_source_language);
}

if (args_info.actual_language == "assembler") {
Expand Down Expand Up @@ -1520,7 +1522,7 @@ process_args(Context& ctx)
DirEntry entry(args_info.output_obj);
if (entry.exists() && !entry.is_regular_file()) {
LOG("Not a regular file: {}", args_info.output_obj);
return Statistic::bad_output_file;
return tl::unexpected(Statistic::bad_output_file);
}
}

Expand All @@ -1531,7 +1533,7 @@ process_args(Context& ctx)
fs::path output_dir = fs::path(args_info.output_obj).parent_path();
if (!output_dir.empty() && !fs::is_directory(output_dir)) {
LOG("Directory does not exist: {}", output_dir);
return Statistic::bad_output_file;
return tl::unexpected(Statistic::bad_output_file);
}

// Some options shouldn't be passed to the real compiler when it compiles
Expand Down Expand Up @@ -1617,7 +1619,7 @@ process_args(Context& ctx)
LOG_RAW(
"-Wp,-M[M]D with -o without -MMD, -MQ or -MT is only supported for"
" GCC or Clang");
return Statistic::unsupported_compiler_option;
return tl::unexpected(Statistic::unsupported_compiler_option);
}
}

Expand Down Expand Up @@ -1744,7 +1746,7 @@ process_args(Context& ctx)
compiler_args.push_back("/showIncludes");
}

return {
return ProcessArgsResult{
preprocessor_args,
extra_args_to_hash,
compiler_args,
Expand Down
31 changes: 3 additions & 28 deletions src/ccache/argprocessing.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
#include <ccache/Args.hpp>
#include <ccache/core/Statistic.hpp>

#include <optional>
#include <tl/expected.hpp>

#include <string>
#include <string_view>
#include <vector>
Expand All @@ -30,16 +31,6 @@ class Context;

struct ProcessArgsResult
{
ProcessArgsResult(core::Statistic error_);
ProcessArgsResult(const Args& preprocessor_args_,
const Args& extra_args_to_hash_,
const Args& compiler_args_,
bool hash_actual_cwd_);

// nullopt on success, otherwise the statistics counter that should be
// incremented.
std::optional<core::Statistic> error;

// Arguments (except -E) to send to the preprocessor.
Args preprocessor_args;

Expand All @@ -53,23 +44,7 @@ struct ProcessArgsResult
bool hash_actual_cwd = false;
};

inline ProcessArgsResult::ProcessArgsResult(core::Statistic error_)
: error(error_)
{
}

inline ProcessArgsResult::ProcessArgsResult(const Args& preprocessor_args_,
const Args& extra_args_to_hash_,
const Args& compiler_args_,
bool hash_actual_cwd_)
: preprocessor_args(preprocessor_args_),
extra_args_to_hash(extra_args_to_hash_),
compiler_args(compiler_args_),
hash_actual_cwd(hash_actual_cwd_)
{
}

ProcessArgsResult process_args(Context& ctx);
tl::expected<ProcessArgsResult, core::Statistic> process_args(Context& ctx);

// Return whether `path` represents a precompiled header (see "Precompiled
// Headers" in GCC docs).
Expand Down
22 changes: 11 additions & 11 deletions src/ccache/ccache.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2490,10 +2490,10 @@ do_cache_compilation(Context& ctx)
// be disabled.
util::setenv("CCACHE_DISABLE", "1");

ProcessArgsResult processed = process_args(ctx);
auto process_args_result = process_args(ctx);

if (processed.error) {
return tl::unexpected(*processed.error);
if (!process_args_result) {
return tl::unexpected(process_args_result.error());
}

TRY(set_up_uncached_err());
Expand Down Expand Up @@ -2595,9 +2595,9 @@ do_cache_compilation(Context& ctx)
}

TRY(hash_common_info(
ctx, processed.preprocessor_args, common_hash, ctx.args_info));
ctx, process_args_result->preprocessor_args, common_hash, ctx.args_info));

if (processed.hash_actual_cwd) {
if (process_args_result->hash_actual_cwd) {
common_hash.hash_delimiter("actual_cwd");
common_hash.hash(ctx.actual_cwd);
}
Expand All @@ -2606,8 +2606,8 @@ do_cache_compilation(Context& ctx)
Hash direct_hash = common_hash;
init_hash_debug(ctx, direct_hash, 'd', "DIRECT MODE", debug_text_file);

Args args_to_hash = processed.preprocessor_args;
args_to_hash.push_back(processed.extra_args_to_hash);
Args args_to_hash = process_args_result->preprocessor_args;
args_to_hash.push_back(process_args_result->extra_args_to_hash);

bool put_result_in_manifest = false;
std::optional<Hash::Digest> result_key;
Expand Down Expand Up @@ -2659,7 +2659,7 @@ do_cache_compilation(Context& ctx)
init_hash_debug(ctx, cpp_hash, 'p', "PREPROCESSOR MODE", debug_text_file);

const auto result_and_manifest_key = calculate_result_and_manifest_key(
ctx, args_to_hash, cpp_hash, &processed.preprocessor_args);
ctx, args_to_hash, cpp_hash, &process_args_result->preprocessor_args);
if (!result_and_manifest_key) {
return tl::unexpected(result_and_manifest_key.error());
}
Expand Down Expand Up @@ -2712,14 +2712,14 @@ do_cache_compilation(Context& ctx)
return tl::unexpected(Statistic::cache_miss);
}

add_prefix(processed.compiler_args, ctx.config.prefix_command());
add_prefix(process_args_result->compiler_args, ctx.config.prefix_command());

// In depend_mode, extend the direct hash.
Hash* depend_mode_hash = ctx.config.depend_mode() ? &direct_hash : nullptr;

// Run real compiler, sending output to cache.
const auto digest =
to_cache(ctx, processed.compiler_args, result_key, depend_mode_hash);
const auto digest = to_cache(
ctx, process_args_result->compiler_args, result_key, depend_mode_hash);
if (!digest) {
return tl::unexpected(digest.error());
}
Expand Down
Loading

0 comments on commit 181ffcf

Please sign in to comment.