diff --git a/common/BUILD b/common/BUILD index 7811bbdc26cf..ef4fc258e334 100644 --- a/common/BUILD +++ b/common/BUILD @@ -116,6 +116,21 @@ cc_library( hdrs = ["concepts.h"], ) +cc_library( + name = "emplace_by_calling", + hdrs = ["emplace_by_calling.h"], +) + +cc_test( + name = "emplace_by_calling_test", + srcs = ["emplace_by_calling_test.cpp"], + deps = [ + ":emplace_by_calling", + "//testing/base:gtest_main", + "@googletest//:gtest", + ], +) + cc_library( name = "enum_base", hdrs = ["enum_base.h"], diff --git a/common/emplace_by_calling.h b/common/emplace_by_calling.h new file mode 100644 index 000000000000..c059a8c68fc8 --- /dev/null +++ b/common/emplace_by_calling.h @@ -0,0 +1,66 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#ifndef CARBON_COMMON_EMPLACE_BY_CALLING_H_ +#define CARBON_COMMON_EMPLACE_BY_CALLING_H_ + +#include +#include + +namespace Carbon { + +// A utility to use when calling an `emplace` function to emplace the result of +// a function call. Expected usage is: +// +// my_widget_vec.emplace_back(EmplaceByCalling([&] { +// return ConstructAWidget(...); +// })); +// +// In this example, the result of `ConstructAWidget` will be constructed +// directly into the new element of `my_widget_vec`, without performing a copy +// or move. +// +// Note that the type of the argument to `emplace_back` is an `EmplaceByCalling` +// instance, not the type `DestT` stored in the container. When the `DestT` +// instance is eventually initialized directly from the `EmplaceByCalling`, a +// conversion function on `EmplaceByCalling` is used that converts to the type +// `DestT` being emplaced. This `DestT` initialization does not call an +// additional `DestT` copy or move constructor to initialize the result, and +// instead initializes it in-place in the container's storage, per the C++17 +// guaranteed copy elision rules. Similarly, within the conversion function, the +// result is initialized directly by calling `make_fn`, again relying on +// guaranteed copy elision. +// +// Because the make function is called from the conversion function, +// `EmplaceByCalling` should only be used in contexts where it will be used to +// initialize a `DestT` object exactly once. This is generally true of `emplace` +// functions. Also, because the `make_fn` callback will be called after the +// container has made space for the new element, it should not inspect or modify +// the container that is being emplaced into. +template +class EmplaceByCalling { + public: + explicit(false) EmplaceByCalling(MakeFnT make_fn) + : make_fn_(std::move(make_fn)) {} + + // Convert to the exact return type of the make function, by calling the make + // function to construct the result. No implicit conversions are permitted + // here, as that would mean we are not constructing the result in place. + template + requires std::same_as> + // NOLINTNEXTLINE(google-explicit-constructor) + explicit(false) operator DestT() && { + return std::move(make_fn_)(); + } + + private: + MakeFnT make_fn_; +}; + +template +EmplaceByCalling(MakeFnT) -> EmplaceByCalling; + +} // namespace Carbon + +#endif // CARBON_COMMON_EMPLACE_BY_CALLING_H_ diff --git a/common/emplace_by_calling_test.cpp b/common/emplace_by_calling_test.cpp new file mode 100644 index 000000000000..ef2160bedef2 --- /dev/null +++ b/common/emplace_by_calling_test.cpp @@ -0,0 +1,62 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include "common/emplace_by_calling.h" + +#include + +#include + +namespace Carbon { +namespace { + +struct NoncopyableType { + NoncopyableType() = default; + NoncopyableType(const NoncopyableType&) = delete; + auto operator=(const NoncopyableType&) -> NoncopyableType& = delete; +}; + +auto Make() -> NoncopyableType { return NoncopyableType(); } + +TEST(EmplaceByCalling, Noncopyable) { + std::list list; + // This should compile. + list.emplace_back(EmplaceByCalling(Make)); +} + +TEST(EmplaceByCalling, NoncopyableInAggregate) { + struct Aggregate { + int a, b, c; + NoncopyableType noncopyable; + }; + + std::list list; + // This should compile. + list.emplace_back(EmplaceByCalling( + [] { return Aggregate{.a = 1, .b = 2, .c = 3, .noncopyable = Make()}; })); +} + +class CopyCounter { + public: + explicit CopyCounter(int* counter) : counter_(counter) {} + CopyCounter(const CopyCounter& other) : counter_(other.counter_) { + ++*counter_; + } + + private: + int* counter_; +}; + +TEST(EmplaceByCalling, NoCopies) { + std::vector vec; + vec.reserve(10); + int copies = 0; + for (int i = 0; i != 10; ++i) { + vec.emplace_back(EmplaceByCalling([&] { return CopyCounter(&copies); })); + } + EXPECT_EQ(0, copies); +} + +} // namespace +} // namespace Carbon diff --git a/toolchain/check/BUILD b/toolchain/check/BUILD index 3731d958ef6c..d4a1e20fe9eb 100644 --- a/toolchain/check/BUILD +++ b/toolchain/check/BUILD @@ -177,6 +177,7 @@ cc_library( ":diagnostic_emitter", ":dump", "//common:check", + "//common:emplace_by_calling", "//common:error", "//common:find", "//common:map", diff --git a/toolchain/check/deferred_definition_worklist.cpp b/toolchain/check/deferred_definition_worklist.cpp index 1e5eb1f4d844..960c3325389b 100644 --- a/toolchain/check/deferred_definition_worklist.cpp +++ b/toolchain/check/deferred_definition_worklist.cpp @@ -6,7 +6,9 @@ #include #include +#include +#include "common/emplace_by_calling.h" #include "common/vlog.h" #include "toolchain/base/kind_switch.h" #include "toolchain/check/handle.h" @@ -27,8 +29,10 @@ auto DeferredDefinitionWorklist::SuspendFunctionAndPush( Parse::FunctionDefinitionStartId node_id) -> void { // TODO: Investigate factoring out `HandleFunctionDefinitionSuspend` to make // `DeferredDefinitionWorklist` reusable. - worklist_.push_back(CheckSkippedDefinition{ - index, HandleFunctionDefinitionSuspend(context, node_id)}); + worklist_.emplace_back(EmplaceByCalling([&] { + return CheckSkippedDefinition{ + index, HandleFunctionDefinitionSuspend(context, node_id)}; + })); CARBON_VLOG("{0}Push CheckSkippedDefinition {1}\n", VlogPrefix, index.index); } @@ -37,96 +41,68 @@ auto DeferredDefinitionWorklist::PushEnterDeferredDefinitionScope( bool nested = !entered_scopes_.empty() && entered_scopes_.back().scope_index == context.decl_name_stack().PeekInitialScopeIndex(); - entered_scopes_.push_back({.worklist_start_index = worklist_.size(), + entered_scopes_.push_back({.nested = nested, + .worklist_start_index = worklist_.size(), .scope_index = context.scope_stack().PeekIndex()}); - worklist_.push_back(EnterDeferredDefinitionScope{ - .suspended_name = std::nullopt, .in_deferred_definition_scope = nested}); - CARBON_VLOG("{0}Push EnterDeferredDefinitionScope {1}\n", VlogPrefix, - nested ? "(nested)" : "(non-nested)"); + if (nested) { + worklist_.emplace_back(EmplaceByCalling([&] { + return EnterNestedDeferredDefinitionScope{.suspended_name = std::nullopt}; + })); + CARBON_VLOG("{0}Push EnterDeferredDefinitionScope (nested)\n", VlogPrefix); + } else { + // Don't push a task to re-enter a non-nested scope. Instead, + // SuspendFinishedScopeAndPush will remain in the scope when executing the + // worklist tasks. + CARBON_VLOG("{0}Entered non-nested deferred definition scope\n", + VlogPrefix); + } return !nested; } auto DeferredDefinitionWorklist::SuspendFinishedScopeAndPush(Context& context) -> FinishedScopeKind { - auto start_index = entered_scopes_.pop_back_val().worklist_start_index; + auto [nested, start_index, _] = entered_scopes_.pop_back_val(); - // If we've not found any deferred definitions in this scope, clean up the - // stack. - if (start_index == worklist_.size() - 1) { + // If we've not found any tasks to perform in this scope, clean up the stack. + // For non-nested scope, there will be no tasks on the worklist for this scope + // in this case; for a nested scope, there will just be a task to re-enter the + // nested scope. + if (!nested && start_index == worklist_.size()) { context.decl_name_stack().PopScope(); - auto enter_scope = - get(worklist_.pop_back_val()); - CARBON_VLOG("{0}Pop EnterDeferredDefinitionScope (empty)\n", VlogPrefix); - return enter_scope.in_deferred_definition_scope - ? FinishedScopeKind::Nested - : FinishedScopeKind::NonNestedEmpty; + CARBON_VLOG("{0}Left non-nested empty deferred definition scope\n", + VlogPrefix); + return FinishedScopeKind::NonNestedEmpty; + } + if (nested && start_index == worklist_.size() - 1) { + CARBON_CHECK(std::holds_alternative( + worklist_.back())); + worklist_.pop_back(); + context.decl_name_stack().PopScope(); + CARBON_VLOG("{0}Pop EnterNestedDeferredDefinitionScope (empty)\n", + VlogPrefix); + return FinishedScopeKind::Nested; } // If we're finishing a nested deferred definition scope, keep track of that // but don't type-check deferred definitions now. - if (auto& enter_scope = - get(worklist_[start_index]); - enter_scope.in_deferred_definition_scope) { + if (nested) { + auto& enter_scope = + get(worklist_[start_index]); // This is a nested deferred definition scope. Suspend the inner scope so we // can restore it when we come to type-check the deferred definitions. - enter_scope.suspended_name = context.decl_name_stack().Suspend(); + enter_scope.suspended_name.emplace( + EmplaceByCalling([&] { return context.decl_name_stack().Suspend(); })); // Enqueue a task to leave the nested scope. - worklist_.push_back( - LeaveDeferredDefinitionScope{.in_deferred_definition_scope = true}); - CARBON_VLOG("{0}Push LeaveDeferredDefinitionScope (nested)\n", VlogPrefix); + worklist_.emplace_back(LeaveNestedDeferredDefinitionScope{}); + CARBON_VLOG("{0}Push LeaveNestedDeferredDefinitionScope\n", VlogPrefix); return FinishedScopeKind::Nested; } - // We're at the end of a non-nested deferred definition scope. Prepare to - // start checking deferred definitions. Enqueue a task to leave this outer - // scope and end checking deferred definitions. - worklist_.push_back( - LeaveDeferredDefinitionScope{.in_deferred_definition_scope = false}); - CARBON_VLOG("{0}Push LeaveDeferredDefinitionScope (non-nested)\n", - VlogPrefix); - - // We'll process the worklist in reverse index order, so reverse the part of - // it we're about to execute so we run our tasks in the order in which they - // were pushed. - std::reverse(worklist_.begin() + start_index, worklist_.end()); - - // Pop the `EnterDeferredDefinitionScope` that's now on the end of the - // worklist. We stay in that scope rather than suspending then immediately - // resuming it. - CARBON_CHECK( - holds_alternative(worklist_.back()), - "Unexpected task in worklist."); - worklist_.pop_back(); - CARBON_VLOG("{0}Handle EnterDeferredDefinitionScope (non-nested)\n", - VlogPrefix); + // We're at the end of a non-nested deferred definition scope. Start checking + // deferred definitions. + CARBON_VLOG("{0}Starting deferred definition processing\n", VlogPrefix); return FinishedScopeKind::NonNestedWithWork; } -auto DeferredDefinitionWorklist::Pop( - llvm::function_refvoid> handle_fn) -> void { - if (vlog_stream_) { - CARBON_KIND_SWITCH(worklist_.back()) { - case CARBON_KIND(const CheckSkippedDefinition& definition): - CARBON_VLOG("{0}Handle CheckSkippedDefinition {1}\n", VlogPrefix, - definition.definition_index.index); - break; - case CARBON_KIND(const EnterDeferredDefinitionScope& enter): - CARBON_CHECK(enter.in_deferred_definition_scope); - CARBON_VLOG("{0}Handle EnterDeferredDefinitionScope (nested)\n", - VlogPrefix); - break; - case CARBON_KIND(const LeaveDeferredDefinitionScope& leave): { - bool nested = leave.in_deferred_definition_scope; - CARBON_VLOG("{0}Handle LeaveDeferredDefinitionScope {1}\n", VlogPrefix, - nested ? "(nested)" : "(non-nested)"); - break; - } - } - } - - handle_fn(std::move(worklist_.back())); - worklist_.pop_back(); -} - } // namespace Carbon::Check diff --git a/toolchain/check/deferred_definition_worklist.h b/toolchain/check/deferred_definition_worklist.h index 97377f9c92b7..b13d6952822e 100644 --- a/toolchain/check/deferred_definition_worklist.h +++ b/toolchain/check/deferred_definition_worklist.h @@ -30,40 +30,35 @@ class DeferredDefinitionWorklist { }; // A worklist task that indicates we should enter a nested deferred definition - // scope. - struct EnterDeferredDefinitionScope { + // scope. We delay processing the contents of nested deferred definition + // scopes until we reach the end of the parent scope. For example: + // + // ``` + // class A { + // class B { + // fn F() -> A { return {}; } + // } + // } // A.B.F is type-checked here, with A complete. + // + // fn F() { + // class C { + // fn G() {} + // } // C.G is type-checked here. + // } + // ``` + struct EnterNestedDeferredDefinitionScope { // The suspended scope. This is only set once we reach the end of the scope. std::optional suspended_name; - // Whether this scope is itself within an outer deferred definition scope. - // If so, we'll delay processing its contents until we reach the end of the - // parent scope. For example: - // - // ``` - // class A { - // class B { - // fn F() -> A { return {}; } - // } - // } // A.B.F is type-checked here, with A complete. - // - // fn F() { - // class C { - // fn G() {} - // } // C.G is type-checked here. - // } - // ``` - bool in_deferred_definition_scope; }; - // A worklist task that indicates we should leave a deferred definition scope. - struct LeaveDeferredDefinitionScope { - // Whether this scope is within another deferred definition scope. - bool in_deferred_definition_scope; - }; + // A worklist task that indicates we should leave a nested deferred definition + // scope. + struct LeaveNestedDeferredDefinitionScope {}; // A pending type-checking task. using Task = - std::variant; + std::variant; explicit DeferredDefinitionWorklist(llvm::raw_ostream* vlog_stream); @@ -94,8 +89,14 @@ class DeferredDefinitionWorklist { // deferred definitions should be type-checked immediately. auto SuspendFinishedScopeAndPush(Context& context) -> FinishedScopeKind; - // Pop and handle the next task on the worklist. - auto Pop(llvm::function_refvoid> handle_fn) -> void; + // Returns the current size of the worklist. + auto size() const -> size_t { return worklist_.size(); } + + // Truncates the worklist to the given size. + auto truncate(int new_size) -> void { worklist_.truncate(new_size); } + + // Gets the given item on the worklist. + auto operator[](int index) -> Task& { return worklist_[index]; } // CHECK that the work list has no further work. auto VerifyEmpty() { @@ -106,7 +107,11 @@ class DeferredDefinitionWorklist { private: // A deferred definition scope that is currently still open. struct EnteredScope { - // The index in worklist_ of the EnterDeferredDefinitionScope task. + // Whether this scope is nested immediately within the enclosing scope. If + // so, deferred definitions are not processed at the end of this scope. + bool nested; + // The index in worklist_ of the first task in this scope. For a nested + // scope, this is a EnterNestedDeferredDefinitionScope task. size_t worklist_start_index; // The corresponding lexical scope index. ScopeIndex scope_index; diff --git a/toolchain/check/node_id_traversal.cpp b/toolchain/check/node_id_traversal.cpp index 0724d23ba05a..b081cbea6a7e 100644 --- a/toolchain/check/node_id_traversal.cpp +++ b/toolchain/check/node_id_traversal.cpp @@ -31,14 +31,22 @@ auto NodeIdTraversal::Next() -> std::optional { // should check, restore its suspended state, and add a corresponding // `Chunk` to the top of the chunk list. if (chunks_.back().checking_deferred_definitions) { - worklist_.Pop([&](DeferredDefinitionWorklist::Task&& task) { - std::visit( - [&](auto&& task) { - PerformTask(std::forward(task)); - }, - std::move(task)); - }); - continue; + if (chunks_.back().next_worklist_index < worklist_.size()) { + std::visit([&](auto& task) { PerformTask(std::move(task)); }, + worklist_[chunks_.back().next_worklist_index++]); + continue; + } + + // Worklist is empty: discard the worklist items associated with this + // chunk, and leave the scope. + worklist_.truncate(chunks_.back().first_worklist_index); + // We reach here when + // `DeferredDefinitionScope::SuspendFinishedScopeAndPush` returns + // `NonNestedWithWork`. In this case it's our responsibility to pop the + // scope left behind by the `Handle*Definition` function for the + // non-nested definition. + context_->decl_name_stack().PopScope(); + chunks_.back().checking_deferred_definitions = false; } // If we're not checking deferred definitions, produce the next parse node @@ -143,23 +151,22 @@ auto NodeIdTraversal::Handle(Parse::NodeKind parse_kind) -> void { if (scope_kind == DeferredDefinitionWorklist::FinishedScopeKind::NonNestedWithWork) { chunks_.back().checking_deferred_definitions = true; + chunks_.back().next_worklist_index = chunks_.back().first_worklist_index; } } } auto NodeIdTraversal::PerformTask( - DeferredDefinitionWorklist::EnterDeferredDefinitionScope&& enter) -> void { + DeferredDefinitionWorklist::EnterNestedDeferredDefinitionScope&& enter) + -> void { CARBON_CHECK(enter.suspended_name, "Entering a scope with no suspension information."); context_->decl_name_stack().Restore(std::move(*enter.suspended_name)); } auto NodeIdTraversal::PerformTask( - DeferredDefinitionWorklist::LeaveDeferredDefinitionScope&& leave) -> void { - if (!leave.in_deferred_definition_scope) { - // We're done with checking deferred definitions. - chunks_.back().checking_deferred_definitions = false; - } + DeferredDefinitionWorklist::LeaveNestedDeferredDefinitionScope&& /*leave*/) + -> void { context_->decl_name_stack().PopScope(); } @@ -175,7 +182,10 @@ auto NodeIdTraversal::PerformTask( definition_info.start_id, definition_info.definition_id); chunks_.push_back({.it = range.begin() + 1, .end = range.end(), - .next_definition = next_deferred_definition_.index()}); + .next_definition = next_deferred_definition_.index(), + .checking_deferred_definitions = false, + .first_worklist_index = worklist_.size(), + .next_worklist_index = worklist_.size()}); ++definition_index.index; next_deferred_definition_.SkipTo(definition_index); } diff --git a/toolchain/check/node_id_traversal.h b/toolchain/check/node_id_traversal.h index 0e3b7c3e5228..65e5d6954ade 100644 --- a/toolchain/check/node_id_traversal.h +++ b/toolchain/check/node_id_traversal.h @@ -62,15 +62,23 @@ class NodeIdTraversal { // them until we're done with this batch of deferred definitions. Otherwise, // we'll pull node IDs from `*it` until it reaches `end`. bool checking_deferred_definitions = false; + // If we're checking deferred definitions, the index of the first task to + // execute from `worklist`. + size_t first_worklist_index; + // If we're checking deferred definitions, the index of the next task to + // execute from `worklist`. + size_t next_worklist_index; }; // Re-enter a nested deferred definition scope. auto PerformTask( - DeferredDefinitionWorklist::EnterDeferredDefinitionScope&& enter) -> void; + DeferredDefinitionWorklist::EnterNestedDeferredDefinitionScope&& enter) + -> void; - // Leave a nested or top-level deferred definition scope. + // Leave a nested deferred definition scope. auto PerformTask( - DeferredDefinitionWorklist::LeaveDeferredDefinitionScope&& leave) -> void; + DeferredDefinitionWorklist::LeaveNestedDeferredDefinitionScope&& leave) + -> void; // Resume checking a deferred definition. auto PerformTask(