Use a Timings* in place of optional<Timings>* (#4607)

The current representation has two nested presence indicatators (the
optional bool, the null pointer). Currently the pointer is never null,
but the style guide suggests that T* should be used for parameters that
may or may not be present, so we do not need the optional here.

> When passing an object's address as an argument, use a reference
> unless one of the following cases applies:
>
> - If the parameter is optional, use a pointer and document that it
>   may be null.

Once the parameter is just a pointer, the ScopedTiming field does not
need an optional either, and can just store the pointer.

It would be more preferable to have an optional representation of a
sometimes-null pointer like optional<T&> to describe a sometimes-null
pointer, as this would allow clearer runtime diagnostics when used
incorrectly (a check failure in unwrapping) and would be better
self-documenting through syntax instead of a comment. But we do not
currently have such a primitive.
This commit is contained in:
Dana Jansens
2024-12-03 19:01:12 +00:00
committed by GitHub
parent e4412a95dd
commit b705be9527
3 changed files with 17 additions and 14 deletions
+2 -2
View File
@@ -449,7 +449,7 @@ class CompilationUnit {
sem_ir_converter_.emplace(node_converters, &*sem_ir_);
return {.consumer = consumer_,
.value_stores = &value_stores_,
.timings = &timings_,
.timings = timings_ ? &*timings_ : nullptr,
.tokens = &*tokens_,
.parse_tree = &*parse_tree_,
.get_parse_tree_and_subtrees = *get_parse_tree_and_subtrees_,
@@ -657,7 +657,7 @@ class CompilationUnit {
llvm::StringLiteral timing_label, llvm::function_ref<void()> fn)
-> void {
CARBON_VLOG("*** {0}: {1} ***\n", logging_label, input_filename_);
Timings::ScopedTiming timing(&timings_, timing_label);
Timings::ScopedTiming timing(timings_ ? &*timings_ : nullptr, timing_label);
fn();
CARBON_VLOG("*** {0} done ***\n", logging_label);
}