Skip to content

Commit

Permalink
Don't attempt to default-construct a reference.
Browse files Browse the repository at this point in the history
Currently, Expected<T> has a bug for references, in that it is default constructible, but references shouldn't be default constructible. (This is hidden because we store a pointer internally).

In VersionedPackageGraph::require_port_version, an ExpectedS<T&> is default constructed, which is in the "impossible" state of neither having a stored T (since references can't be default constructed), but also not being assigned an error state.

This change hoists the only place where the ExpectedS could contain an error into an early-return, allowing a plain pointer to be used, and avoiding constructing the impossible thing.

(The bug in Expected that allowed this to happen is fixed in microsoft#511 , but this change was tricky enough I thought extracting it for review alone made sense)
  • Loading branch information
BillyONeal committed May 25, 2022
1 parent 3f75bc9 commit 524130d
Showing 1 changed file with 45 additions and 45 deletions.
90 changes: 45 additions & 45 deletions src/vcpkg/dependencies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1551,79 +1551,79 @@ namespace vcpkg::Dependencies
const Version& version,
const std::string& origin)
{
ExpectedS<const vcpkg::SourceControlFileAndLocation&> maybe_scfl;

// if this port is an overlay port, ignore the given version and use the version from the overlay
auto maybe_overlay = m_o_provider.get_control_file(graph_entry.first.name());
if (auto p_overlay = maybe_overlay.get())
const vcpkg::SourceControlFileAndLocation* p_scfl = maybe_overlay.get();
if (p_scfl)
{
const auto overlay_version = p_overlay->source_control_file->to_version();
const auto overlay_version = p_scfl->source_control_file->to_version();
// If the original request did not match the overlay version, restart this function to operate on the
// overlay version
if (version != overlay_version)
{
return require_port_version(graph_entry, overlay_version, origin);
require_port_version(graph_entry, overlay_version, origin);
return;
}
maybe_scfl = *p_overlay;
}
else
{
// if there is a override, ignore the given version and use the version from the override
auto over_it = m_overrides.find(graph_entry.first.name());
if (over_it != m_overrides.end() && over_it->second != version)
{
return require_port_version(graph_entry, over_it->second, origin);
require_port_version(graph_entry, over_it->second, origin);
return;
}
maybe_scfl = m_ver_provider.get_control_file({graph_entry.first.name(), version});
}

if (auto p_scfl = maybe_scfl.get())
{
auto& versioned_graph_entry = graph_entry.second.emplace_node(
p_scfl->source_control_file->core_paragraph->version_scheme, version);
versioned_graph_entry.origins.push_back(origin);
// Use the new source control file if we currently don't have one or the new one is newer
bool replace;
if (versioned_graph_entry.scfl == nullptr)
{
replace = true;
}
else if (versioned_graph_entry.scfl == p_scfl)
const auto maybe_scfl = m_ver_provider.get_control_file({graph_entry.first.name(), version});
p_scfl = maybe_scfl.get();
if (!p_scfl)
{
replace = false;
m_errors.push_back(maybe_scfl.error());
return;
}
else
}

auto& versioned_graph_entry = graph_entry.second.emplace_node(
p_scfl->source_control_file->core_paragraph->version_scheme, version);
versioned_graph_entry.origins.push_back(origin);
// Use the new source control file if we currently don't have one or the new one is newer
bool replace;
if (versioned_graph_entry.scfl == nullptr)
{
replace = true;
}
else if (versioned_graph_entry.scfl == p_scfl)
{
replace = false;
}
else
{
replace = versioned_graph_entry.is_less_than(version);
}

if (replace)
{
versioned_graph_entry.scfl = p_scfl;
versioned_graph_entry.version = p_scfl->source_control_file->to_version();
versioned_graph_entry.deps.clear();

// add all dependencies to the graph
add_feature_to(graph_entry, versioned_graph_entry, "core");

for (auto&& f : graph_entry.second.requested_features)
{
replace = versioned_graph_entry.is_less_than(version);
add_feature_to(graph_entry, versioned_graph_entry, f);
}

if (replace)
if (graph_entry.second.default_features)
{
versioned_graph_entry.scfl = p_scfl;
versioned_graph_entry.version = p_scfl->source_control_file->to_version();
versioned_graph_entry.deps.clear();

// add all dependencies to the graph
add_feature_to(graph_entry, versioned_graph_entry, "core");

for (auto&& f : graph_entry.second.requested_features)
for (auto&& f : p_scfl->source_control_file->core_paragraph->default_features)
{
add_feature_to(graph_entry, versioned_graph_entry, f);
}

if (graph_entry.second.default_features)
{
for (auto&& f : p_scfl->source_control_file->core_paragraph->default_features)
{
add_feature_to(graph_entry, versioned_graph_entry, f);
}
}
}
}
else
{
m_errors.push_back(maybe_scfl.error());
}
}

void VersionedPackageGraph::require_port_defaults(std::pair<const PackageSpec, PackageNode>& ref,
Expand Down

0 comments on commit 524130d

Please sign in to comment.