Skip to content

Commit

Permalink
Remove comparator.hh and switch to <=> in a bunch of places
Browse files Browse the repository at this point in the history
Known behavior changes:

- `MemorySourceAccessor`'s comparison operators no longer forget to
  compare the `SourceAccessor` base class.

Progress on #10832

What remains for that issue is hopefully much easier!
  • Loading branch information
Ericson2314 committed Jul 12, 2024
1 parent 2a95a2d commit 7d84ab9
Show file tree
Hide file tree
Showing 49 changed files with 282 additions and 275 deletions.
40 changes: 16 additions & 24 deletions src/libcmd/built-path.cc
Original file line number Diff line number Diff line change
@@ -1,37 +1,29 @@
#include "built-path.hh"
#include "derivations.hh"
#include "store-api.hh"
#include "comparator.hh"

#include <nlohmann/json.hpp>

#include <optional>

namespace nix {

#define CMP_ONE(CHILD_TYPE, MY_TYPE, FIELD, COMPARATOR) \
bool MY_TYPE ::operator COMPARATOR (const MY_TYPE & other) const \
{ \
const MY_TYPE* me = this; \
auto fields1 = std::tie(*me->drvPath, me->FIELD); \
me = &other; \
auto fields2 = std::tie(*me->drvPath, me->FIELD); \
return fields1 COMPARATOR fields2; \
}
#define CMP(CHILD_TYPE, MY_TYPE, FIELD) \
CMP_ONE(CHILD_TYPE, MY_TYPE, FIELD, ==) \
CMP_ONE(CHILD_TYPE, MY_TYPE, FIELD, !=) \
CMP_ONE(CHILD_TYPE, MY_TYPE, FIELD, <)

#define FIELD_TYPE std::pair<std::string, StorePath>
CMP(SingleBuiltPath, SingleBuiltPathBuilt, output)
#undef FIELD_TYPE

#define FIELD_TYPE std::map<std::string, StorePath>
CMP(SingleBuiltPath, BuiltPathBuilt, outputs)
#undef FIELD_TYPE

#undef CMP
#undef CMP_ONE
// Custom implementation to avoid `ref` ptr equality
GENERATE_CMP_EXT(
,
std::strong_ordering,
SingleBuiltPathBuilt,
*me->drvPath,
me->output);

// Custom implementation to avoid `ref` ptr equality
GENERATE_CMP_EXT(
,
std::strong_ordering,
BuiltPathBuilt,
*me->drvPath,
me->outputs);

StorePath SingleBuiltPath::outPath() const
{
Expand Down
14 changes: 11 additions & 3 deletions src/libcmd/built-path.hh
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ struct SingleBuiltPathBuilt {
static SingleBuiltPathBuilt parse(const StoreDirConfig & store, std::string_view, std::string_view);
nlohmann::json toJSON(const StoreDirConfig & store) const;

DECLARE_CMP(SingleBuiltPathBuilt);
bool operator ==(const SingleBuiltPathBuilt &) const noexcept;
std::strong_ordering operator <=>(const SingleBuiltPathBuilt &) const noexcept;
};

using _SingleBuiltPathRaw = std::variant<
Expand All @@ -33,6 +34,9 @@ struct SingleBuiltPath : _SingleBuiltPathRaw {
using Opaque = DerivedPathOpaque;
using Built = SingleBuiltPathBuilt;

bool operator == (const SingleBuiltPath &) const = default;
auto operator <=> (const SingleBuiltPath &) const = default;

inline const Raw & raw() const {
return static_cast<const Raw &>(*this);
}
Expand All @@ -59,11 +63,12 @@ struct BuiltPathBuilt {
ref<SingleBuiltPath> drvPath;
std::map<std::string, StorePath> outputs;

bool operator == (const BuiltPathBuilt &) const noexcept;
std::strong_ordering operator <=> (const BuiltPathBuilt &) const noexcept;

std::string to_string(const StoreDirConfig & store) const;
static BuiltPathBuilt parse(const StoreDirConfig & store, std::string_view, std::string_view);
nlohmann::json toJSON(const StoreDirConfig & store) const;

DECLARE_CMP(BuiltPathBuilt);
};

using _BuiltPathRaw = std::variant<
Expand All @@ -82,6 +87,9 @@ struct BuiltPath : _BuiltPathRaw {
using Opaque = DerivedPathOpaque;
using Built = BuiltPathBuilt;

bool operator == (const BuiltPath &) const = default;
auto operator <=> (const BuiltPath &) const = default;

inline const Raw & raw() const {
return static_cast<const Raw &>(*this);
}
Expand Down
2 changes: 1 addition & 1 deletion src/libexpr-c/nix_api_external.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ class NixCExternalValue : public nix::ExternalValueBase
/**
* Compare to another value of the same type.
*/
virtual bool operator==(const ExternalValueBase & b) const override
virtual bool operator==(const ExternalValueBase & b) const noexcept override
{
if (!desc.equal) {
return false;
Expand Down
4 changes: 2 additions & 2 deletions src/libexpr/attr-set.hh
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ struct Attr
Attr(Symbol name, Value * value, PosIdx pos = noPos)
: name(name), pos(pos), value(value) { };
Attr() { };
bool operator < (const Attr & a) const
auto operator <=> (const Attr & a) const
{
return name < a.name;
return name <=> a.name;
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2842,7 +2842,7 @@ std::string ExternalValueBase::coerceToString(EvalState & state, const PosIdx &
}


bool ExternalValueBase::operator==(const ExternalValueBase & b) const
bool ExternalValueBase::operator==(const ExternalValueBase & b) const noexcept
{
return false;
}
Expand Down
9 changes: 2 additions & 7 deletions src/libexpr/pos-idx.hh
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,15 @@ public:
return id > 0;
}

bool operator<(const PosIdx other) const
auto operator<=>(const PosIdx other) const
{
return id < other.id;
return id <=> other.id;
}

bool operator==(const PosIdx other) const
{
return id == other.id;
}

bool operator!=(const PosIdx other) const
{
return id != other.id;
}
};

inline PosIdx noPos = {};
Expand Down
3 changes: 1 addition & 2 deletions src/libexpr/symbol-table.hh
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,8 @@ public:

explicit operator bool() const { return id > 0; }

bool operator<(const Symbol other) const { return id < other.id; }
auto operator<=>(const Symbol other) const { return id <=> other.id; }
bool operator==(const Symbol other) const { return id == other.id; }
bool operator!=(const Symbol other) const { return id != other.id; }
};

/**
Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/value.hh
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class ExternalValueBase
* Compare to another value of the same type. Defaults to uncomparable,
* i.e. always false.
*/
virtual bool operator ==(const ExternalValueBase & b) const;
virtual bool operator ==(const ExternalValueBase & b) const noexcept;

/**
* Print the value as JSON. Defaults to unconvertable, i.e. throws an error
Expand Down
2 changes: 1 addition & 1 deletion src/libfetchers/fetchers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ Attrs Input::toAttrs() const
return attrs;
}

bool Input::operator ==(const Input & other) const
bool Input::operator ==(const Input & other) const noexcept
{
return attrs == other.attrs;
}
Expand Down
2 changes: 1 addition & 1 deletion src/libfetchers/fetchers.hh
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public:
*/
bool isLocked() const;

bool operator ==(const Input & other) const;
bool operator ==(const Input & other) const noexcept;

bool contains(const Input & other) const;

Expand Down
5 changes: 0 additions & 5 deletions src/libflake/flake/flakeref.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,6 @@ std::ostream & operator << (std::ostream & str, const FlakeRef & flakeRef)
return str;
}

bool FlakeRef::operator ==(const FlakeRef & other) const
{
return input == other.input && subdir == other.subdir;
}

FlakeRef FlakeRef::resolve(ref<Store> store) const
{
auto [input2, extraAttrs] = lookupInRegistries(store, input);
Expand Down
2 changes: 1 addition & 1 deletion src/libflake/flake/flakeref.hh
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ struct FlakeRef
*/
Path subdir;

bool operator==(const FlakeRef & other) const;
bool operator ==(const FlakeRef & other) const = default;

FlakeRef(fetchers::Input && input, const Path & subdir)
: input(std::move(input)), subdir(subdir)
Expand Down
5 changes: 0 additions & 5 deletions src/libflake/flake/lockfile.cc
Original file line number Diff line number Diff line change
Expand Up @@ -249,11 +249,6 @@ bool LockFile::operator ==(const LockFile & other) const
return toJSON().first == other.toJSON().first;
}

bool LockFile::operator !=(const LockFile & other) const
{
return !(*this == other);
}

InputPath parseInputPath(std::string_view s)
{
InputPath path;
Expand Down
3 changes: 0 additions & 3 deletions src/libflake/flake/lockfile.hh
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,6 @@ struct LockFile
std::optional<FlakeRef> isUnlocked() const;

bool operator ==(const LockFile & other) const;
// Needed for old gcc versions that don't synthesize it (like gcc 8.2.2
// that is still the default on aarch64-linux)
bool operator !=(const LockFile & other) const;

std::shared_ptr<Node> findInput(const InputPath & path);

Expand Down
14 changes: 2 additions & 12 deletions src/libstore/build-result.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,7 @@

namespace nix {

GENERATE_CMP_EXT(
,
BuildResult,
me->status,
me->errorMsg,
me->timesBuilt,
me->isNonDeterministic,
me->builtOutputs,
me->startTime,
me->stopTime,
me->cpuUser,
me->cpuSystem);
bool BuildResult::operator==(const BuildResult &) const noexcept = default;
std::strong_ordering BuildResult::operator<=>(const BuildResult &) const noexcept = default;

}
4 changes: 2 additions & 2 deletions src/libstore/build-result.hh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

#include "realisation.hh"
#include "derived-path.hh"
#include "comparator.hh"

#include <string>
#include <chrono>
Expand Down Expand Up @@ -101,7 +100,8 @@ struct BuildResult
*/
std::optional<std::chrono::microseconds> cpuUser, cpuSystem;

DECLARE_CMP(BuildResult);
bool operator ==(const BuildResult &) const noexcept;
std::strong_ordering operator <=>(const BuildResult &) const noexcept;

bool success()
{
Expand Down
18 changes: 18 additions & 0 deletions src/libstore/content-address.hh
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ struct ContentAddressMethod

Raw raw;

bool operator ==(const ContentAddressMethod &) const = default;
auto operator <=>(const ContentAddressMethod &) const = default;

MAKE_WRAPPER_CONSTRUCTOR(ContentAddressMethod);
Expand Down Expand Up @@ -159,6 +160,7 @@ struct ContentAddress
*/
Hash hash;

bool operator ==(const ContentAddress &) const = default;
auto operator <=>(const ContentAddress &) const = default;

/**
Expand Down Expand Up @@ -217,6 +219,10 @@ struct StoreReferences
* iff self is true.
*/
size_t size() const;

bool operator ==(const StoreReferences &) const = default;
// TODO Darwin missing `std::map::operator <=>`, can't do yet.
//auto operator <=>(const StoreReferences &) const = default;
};

// This matches the additional info that we need for makeTextPath
Expand All @@ -232,6 +238,10 @@ struct TextInfo
* disallowed
*/
StorePathSet references;

bool operator ==(const TextInfo &) const = default;
// TODO Darwin missing `std::map::operator <=>`, can't do yet.
//auto operator <=>(const TextInfo &) const = default;
};

struct FixedOutputInfo
Expand All @@ -250,6 +260,10 @@ struct FixedOutputInfo
* References to other store objects or this one.
*/
StoreReferences references;

bool operator ==(const FixedOutputInfo &) const = default;
// TODO Darwin missing `std::map::operator <=>`, can't do yet.
//auto operator <=>(const FixedOutputInfo &) const = default;
};

/**
Expand All @@ -266,6 +280,10 @@ struct ContentAddressWithReferences

Raw raw;

bool operator ==(const ContentAddressWithReferences &) const = default;
// TODO Darwin missing `std::map::operator <=>`, can't do yet.
//auto operator <=>(const ContentAddressWithReferences &) const = default;

MAKE_WRAPPER_CONSTRUCTOR(ContentAddressWithReferences);

/**
Expand Down
Loading

0 comments on commit 7d84ab9

Please sign in to comment.