refactor(tvix/libutil): Mark single-argument constructors explicit

This is the clang-tidy lint 'google-explicit-constructor'.

There's a whole bunch of breakage that was introduced by this, and we
had to opt out a few types of this (esp. the string formatting crap).

In some cases minor other changes have been done to keep the code
working, instead of converting between types (e.g. an explicit
comparison operator implementation for nix::Pid).

Change-Id: I12e1ca51a6bc2c882dba81a2526b9729d26988e7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1832
Tested-by: BuildkiteCI
Reviewed-by: kanepyork <rikingcoding@gmail.com>
Reviewed-by: glittershark <grfn@gws.fyi>

third_party/nix/src/libutil/archive.cc

@@ -45,7 +45,7 @@ PathFilter defaultPathFilter = [](const Path& /*unused*/) { return true; };
 static void dumpContents(const Path& path, size_t size, Sink& sink) {
   sink << "contents" << size;
 
-  AutoCloseFD fd = open(path.c_str(), O_RDONLY | O_CLOEXEC);
+  AutoCloseFD fd(open(path.c_str(), O_RDONLY | O_CLOEXEC));
   if (!fd) {
     throw SysError(format("opening file '%1%'") % path);
   }
@@ -334,7 +334,8 @@ struct RestoreSink : ParseSink {
 
   void createRegularFile(const Path& path) override {
     Path p = dstPath + path;
-    fd = open(p.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0666);
+    fd = AutoCloseFD(
+        open(p.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0666));
     if (!fd) {
       throw SysError(format("creating file '%1%'") % p);
     }

third_party/nix/src/libutil/archive.hh

@@ -62,7 +62,7 @@ struct ParseSink {
 struct TeeSink : ParseSink {
   TeeSource source;
 
-  TeeSink(Source& source) : source(source) {}
+  explicit TeeSink(Source& source) : source(source) {}
 };
 
 void parseDump(ParseSink& sink, Source& source);

third_party/nix/src/libutil/args.hh

@@ -65,7 +65,8 @@ class Args {
     Args& args;
     Flag::ptr flag;
     friend class Args;
-    FlagMaker(Args& args) : args(args), flag(std::make_shared<Flag>()){};
+    explicit FlagMaker(Args& args)
+        : args(args), flag(std::make_shared<Flag>()){};
 
    public:
     ~FlagMaker();

third_party/nix/src/libutil/config.hh

@@ -16,7 +16,8 @@ class AbstractConfig {
  protected:
   StringMap unknownSettings;
 
-  AbstractConfig(const StringMap& initials = {}) : unknownSettings(initials) {}
+  explicit AbstractConfig(const StringMap& initials = {})
+      : unknownSettings(initials) {}
 
  public:
   virtual bool set(const std::string& name, const std::string& value) = 0;
@@ -74,7 +75,7 @@ class Config : public AbstractConfig {
   Settings _settings;
 
  public:
-  Config(const StringMap& initials = {}) : AbstractConfig(initials) {}
+  explicit Config(const StringMap& initials = {}) : AbstractConfig(initials) {}
 
   bool set(const std::string& name, const std::string& value) override;
 
@@ -218,7 +219,7 @@ struct GlobalConfig : public AbstractConfig {
   void convertToArgs(Args& args, const std::string& category) override;
 
   struct Register {
-    Register(Config* config);
+    explicit Register(Config* config);
   };
 };
 

third_party/nix/src/libutil/finally.hh

@@ -8,6 +8,6 @@ class Finally {
   std::function<void()> fun;
 
  public:
-  Finally(std::function<void()> fun) : fun(fun) {}
+  explicit Finally(std::function<void()> fun) : fun(fun) {}
   ~Finally() { fun(); }
 };

third_party/nix/src/libutil/hash.cc

@@ -387,7 +387,7 @@ Hash hashFile(HashType ht, const Path& path) {
   Hash hash(ht);
   start(ht, ctx);
 
-  AutoCloseFD fd = open(path.c_str(), O_RDONLY | O_CLOEXEC);
+  AutoCloseFD fd(open(path.c_str(), O_RDONLY | O_CLOEXEC));
   if (!fd) {
     throw SysError(format("opening file '%1%'") % path);
   }

third_party/nix/src/libutil/hash.hh

@@ -42,14 +42,14 @@ struct Hash {
   Hash(){};
 
   /* Create a zero-filled hash object. */
-  Hash(HashType type) : type(type) { init(); };
+  explicit Hash(HashType type) : type(type) { init(); };
 
   /* Initialize the hash from a string representation, in the format
      "[<type>:]<base16|base32|base64>" or "<type>-<base64>" (a
      Subresource Integrity hash expression). If the 'type' argument
      is htUnknown, then the hash type must be specified in the
      string. */
-  Hash(std::string_view s, HashType type = htUnknown);
+  explicit Hash(std::string_view s, HashType type = htUnknown);
 
   /* Status-returning version of above constructor */
   static absl::StatusOr<Hash> deserialize(std::string_view s,
@@ -61,7 +61,7 @@ struct Hash {
   void init();
 
   /* Check whether a hash is set. */
-  operator bool() const { return type != htUnknown; }
+  explicit operator bool() const { return type != htUnknown; }
 
   /* Check whether two hash are equal. */
   bool operator==(const Hash& h2) const;
@@ -136,7 +136,7 @@ class HashSink : public BufferedSink {
   unsigned long long bytes;
 
  public:
-  HashSink(HashType ht);
+  explicit HashSink(HashType ht);
   HashSink(const HashSink& h);
   ~HashSink();
   void write(const unsigned char* data, size_t len);

third_party/nix/src/libutil/json.hh

@@ -29,7 +29,7 @@ class JSONWriter {
 
   JSONWriter(std::ostream& str, bool indent);
 
-  JSONWriter(JSONState* state);
+  explicit JSONWriter(JSONState* state);
 
   ~JSONWriter();
 
@@ -50,10 +50,11 @@ class JSONList : JSONWriter {
 
   void open();
 
-  JSONList(JSONState* state) : JSONWriter(state) { open(); }
+  explicit JSONList(JSONState* state) : JSONWriter(state) { open(); }
 
  public:
-  JSONList(std::ostream& str, bool indent = false) : JSONWriter(str, indent) {
+  explicit JSONList(std::ostream& str, bool indent = false)
+      : JSONWriter(str, indent) {
     open();
   }
 
@@ -80,12 +81,13 @@ class JSONObject : JSONWriter {
 
   void open();
 
-  JSONObject(JSONState* state) : JSONWriter(state) { open(); }
+  explicit JSONObject(JSONState* state) : JSONWriter(state) { open(); }
 
   void attr(const std::string& s);
 
  public:
-  JSONObject(std::ostream& str, bool indent = false) : JSONWriter(str, indent) {
+  explicit JSONObject(std::ostream& str, bool indent = false)
+      : JSONWriter(str, indent) {
     open();
   }
 
@@ -114,7 +116,7 @@ class JSONPlaceholder : JSONWriter {
   friend class JSONList;
   friend class JSONObject;
 
-  JSONPlaceholder(JSONState* state) : JSONWriter(state) {}
+  explicit JSONPlaceholder(JSONState* state) : JSONWriter(state) {}
 
   void assertValid() {
     assertActive();
@@ -122,7 +124,7 @@ class JSONPlaceholder : JSONWriter {
   }
 
  public:
-  JSONPlaceholder(std::ostream& str, bool indent = false)
+  explicit JSONPlaceholder(std::ostream& str, bool indent = false)
       : JSONWriter(str, indent) {}
 
   ~JSONPlaceholder() { assert(!first || std::uncaught_exception()); }

third_party/nix/src/libutil/lazy.hh

@@ -25,7 +25,7 @@ class Lazy {
   std::exception_ptr ex;
 
  public:
-  Lazy(Init init) : init(init) {}
+  explicit Lazy(Init init) : init(init) {}
 
   const T& operator()() {
     std::call_once(done, [&]() {

third_party/nix/src/libutil/lru-cache.hh

@@ -27,7 +27,7 @@ class LRUCache {
   LRU lru;
 
  public:
-  LRUCache(size_t capacity) : capacity(capacity) {}
+  explicit LRUCache(size_t capacity) : capacity(capacity) {}
 
   /* Insert or upsert an item in the cache. */
   void upsert(const Key& key, const Value& value) {

third_party/nix/src/libutil/pool.hh

@@ -53,7 +53,7 @@ class Pool {
   std::condition_variable wakeup;
 
  public:
-  Pool(
+  explicit Pool(
       size_t max = std::numeric_limits<size_t>::max(),
       const Factory& factory = []() { return make_ref<R>(); },
       const Validator& validator = [](ref<R> r) { return true; })

third_party/nix/src/libutil/ref.hh

@@ -9,7 +9,7 @@ namespace nix {
 /* A simple non-nullable reference-counted pointer. Actually a wrapper
    around std::shared_ptr that prevents non-null constructions. */
 template <typename T>
-class ref {
+class ref {  // TODO(tazjin): rename to brainworm_ref or something
  private:
   std::shared_ptr<T> p;
 

third_party/nix/src/libutil/serialise.hh

@@ -23,7 +23,7 @@ struct BufferedSink : Sink {
   size_t bufSize, bufPos;
   std::unique_ptr<unsigned char[]> buffer;
 
-  BufferedSink(size_t bufSize = 32 * 1024)
+  explicit BufferedSink(size_t bufSize = 32 * 1024)
       : bufSize(bufSize), bufPos(0), buffer(nullptr) {}
 
   void operator()(const unsigned char* data, size_t len) override;
@@ -59,7 +59,7 @@ struct BufferedSource : Source {
   size_t bufSize, bufPosIn, bufPosOut;
   std::unique_ptr<unsigned char[]> buffer;
 
-  BufferedSource(size_t bufSize = 32 * 1024)
+  explicit BufferedSource(size_t bufSize = 32 * 1024)
       : bufSize(bufSize), bufPosIn(0), bufPosOut(0), buffer(nullptr) {}
 
   size_t read(unsigned char* data, size_t len) override;
@@ -78,7 +78,7 @@ struct FdSink : BufferedSink {
   size_t written = 0;
 
   FdSink() : fd(-1) {}
-  FdSink(int fd) : fd(fd) {}
+  explicit FdSink(int fd) : fd(fd) {}
   FdSink(FdSink&&) = default;
 
   FdSink& operator=(FdSink&& s) {
@@ -106,7 +106,7 @@ struct FdSource : BufferedSource {
   size_t read = 0;
 
   FdSource() : fd(-1) {}
-  FdSource(int fd) : fd(fd) {}
+  explicit FdSource(int fd) : fd(fd) {}
   FdSource(FdSource&&) = default;
 
   FdSource& operator=(FdSource&& s) {
@@ -129,7 +129,7 @@ struct FdSource : BufferedSource {
 struct StringSink : Sink {
   ref<std::string> s;
   StringSink() : s(make_ref<std::string>()){};
-  StringSink(ref<std::string> s) : s(s){};
+  explicit StringSink(ref<std::string> s) : s(s){};
   void operator()(const unsigned char* data, size_t len) override;
 };
 
@@ -137,7 +137,7 @@ struct StringSink : Sink {
 struct StringSource : Source {
   const std::string& s;
   size_t pos;
-  StringSource(const std::string& _s) : s(_s), pos(0) {}
+  explicit StringSource(const std::string& _s) : s(_s), pos(0) {}
   size_t read(unsigned char* data, size_t len) override;
 };
 
@@ -145,7 +145,8 @@ struct StringSource : Source {
 struct TeeSource : Source {
   Source& orig;
   ref<std::string> data;
-  TeeSource(Source& orig) : orig(orig), data(make_ref<std::string>()) {}
+  explicit TeeSource(Source& orig)
+      : orig(orig), data(make_ref<std::string>()) {}
   size_t read(unsigned char* data, size_t len) {
     size_t n = orig.read(data, len);
     this->data->append((const char*)data, n);
@@ -186,7 +187,7 @@ struct LambdaSink : Sink {
 
   lambda_t lambda;
 
-  LambdaSink(const lambda_t& lambda) : lambda(lambda) {}
+  explicit LambdaSink(const lambda_t& lambda) : lambda(lambda) {}
 
   virtual void operator()(const unsigned char* data, size_t len) {
     lambda(data, len);
@@ -199,7 +200,7 @@ struct LambdaSource : Source {
 
   lambda_t lambda;
 
-  LambdaSource(const lambda_t& lambda) : lambda(lambda) {}
+  explicit LambdaSource(const lambda_t& lambda) : lambda(lambda) {}
 
   size_t read(unsigned char* data, size_t len) override {
     return lambda(data, len);

third_party/nix/src/libutil/sync.hh

@@ -31,15 +31,15 @@ class Sync {
 
  public:
   Sync() {}
-  Sync(const T& data) : data(data) {}
-  Sync(T&& data) noexcept : data(std::move(data)) {}
+  explicit Sync(const T& data) : data(data) {}
+  explicit Sync(T&& data) noexcept : data(std::move(data)) {}
 
   class Lock {
    private:
     Sync* s;
     std::unique_lock<M> lk;
     friend Sync;
-    Lock(Sync* s) : s(s), lk(s->mutex) {}
+    explicit Lock(Sync* s) : s(s), lk(s->mutex) {}
 
    public:
     Lock(Lock&& l) : s(l.s) { abort(); }

third_party/nix/src/libutil/thread-pool.hh

@@ -17,7 +17,7 @@ MakeError(ThreadPoolShutDown, Error);
    (lambdas). */
 class ThreadPool {
  public:
-  ThreadPool(size_t maxThreads = 0);
+  explicit ThreadPool(size_t maxThreads = 0);
 
   ~ThreadPool();
 

third_party/nix/src/libutil/types.hh

@@ -27,7 +27,7 @@ using boost::format;
    for all variadic arguments but ignoring the result. */
 struct nop {
   template <typename... T>
-  nop(T...) {}
+  explicit nop(T...) {}
 };
 
 struct FormatOrString {
@@ -69,11 +69,11 @@ class BaseError : public std::exception {
   unsigned int status = 1;  // exit status
 
   template <typename... Args>
-  BaseError(unsigned int status, Args... args)
+  explicit BaseError(unsigned int status, Args... args)
       : err(fmt(args...)), status(status) {}
 
   template <typename... Args>
-  BaseError(Args... args) : err(fmt(args...)) {}
+  explicit BaseError(Args... args) : err(fmt(args...)) {}
 
 #ifdef EXCEPTION_NEEDS_THROW_SPEC
   ~BaseError() noexcept {};
@@ -100,7 +100,7 @@ class SysError : public Error {
   int errNo;
 
   template <typename... Args>
-  SysError(Args... args) : Error(addErrno(fmt(args...))) {}
+  explicit SysError(Args... args) : Error(addErrno(fmt(args...))) {}
 
  private:
   std::string addErrno(const std::string& s);

third_party/nix/src/libutil/util.cc

@@ -312,7 +312,7 @@ std::string readFile(int fd) {
 }
 
 std::string readFile(absl::string_view path, bool drain) {
-  AutoCloseFD fd = open(std::string(path).c_str(), O_RDONLY | O_CLOEXEC);
+  AutoCloseFD fd(open(std::string(path).c_str(), O_RDONLY | O_CLOEXEC));
   if (!fd) {
     throw SysError(format("opening file '%1%'") % path);
   }
@@ -321,7 +321,7 @@ std::string readFile(absl::string_view path, bool drain) {
 
 void readFile(absl::string_view path, Sink& sink) {
   // TODO(tazjin): use stdlib functions for this stuff
-  AutoCloseFD fd = open(std::string(path).c_str(), O_RDONLY | O_CLOEXEC);
+  AutoCloseFD fd(open(std::string(path).c_str(), O_RDONLY | O_CLOEXEC));
   if (!fd) {
     throw SysError("opening file '%s'", path);
   }
@@ -329,8 +329,8 @@ void readFile(absl::string_view path, Sink& sink) {
 }
 
 void writeFile(const Path& path, const std::string& s, mode_t mode) {
-  AutoCloseFD fd =
-      open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC, mode);
+  AutoCloseFD fd(
+      open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC, mode));
   if (!fd) {
     throw SysError(format("opening file '%1%'") % path);
   }
@@ -338,8 +338,8 @@ void writeFile(const Path& path, const std::string& s, mode_t mode) {
 }
 
 void writeFile(const Path& path, Source& source, mode_t mode) {
-  AutoCloseFD fd =
-      open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC, mode);
+  AutoCloseFD fd(
+      open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC, mode));
   if (!fd) {
     throw SysError(format("opening file '%1%'") % path);
   }
@@ -790,8 +790,8 @@ void Pipe::create() {
   closeOnExec(fds[0]);
   closeOnExec(fds[1]);
 #endif
-  readSide = fds[0];
-  writeSide = fds[1];
+  readSide = AutoCloseFD(fds[0]);
+  writeSide = AutoCloseFD(fds[1]);
 }
 
 //////////////////////////////////////////////////////////////////////
@@ -868,7 +868,7 @@ void killUser(uid_t uid) {
 
   ProcessOptions options;
 
-  Pid pid = startProcess(
+  Pid pid(startProcess(
       [&]() {
         if (setuid(uid) == -1) {
           throw SysError("setting uid");
@@ -888,7 +888,7 @@ void killUser(uid_t uid) {
 
         _exit(0);
       },
-      options);
+      options));
 
   int status = pid.wait();
   if (status != 0) {
@@ -1024,7 +1024,7 @@ void runProgram2(const RunOptions& options) {
   ProcessOptions processOptions;
 
   /* Fork. */
-  Pid pid = startProcess(
+  Pid pid(startProcess(
       [&]() {
         if (options.environment) {
           replaceEnv(*options.environment);
@@ -1070,9 +1070,9 @@ void runProgram2(const RunOptions& options) {
 
         throw SysError("executing '%1%'", options.program);
       },
-      processOptions);
+      processOptions));
 
-  out.writeSide = -1;
+  out.writeSide = AutoCloseFD(-1);
 
   std::thread writerThread;
 
@@ -1085,7 +1085,7 @@ void runProgram2(const RunOptions& options) {
   });
 
   if (source != nullptr) {
-    in.readSide = -1;
+    in.readSide = AutoCloseFD(-1);
     writerThread = std::thread([&]() {
       try {
         std::vector<unsigned char> buf(8 * 1024);
@@ -1102,7 +1102,7 @@ void runProgram2(const RunOptions& options) {
       } catch (...) {
         promise.set_exception(std::current_exception());
       }
-      in.writeSide = -1;
+      in.writeSide = AutoCloseFD(-1);
     });
   }
 

third_party/nix/src/libutil/util.hh

@@ -167,11 +167,11 @@ class AutoDelete {
 
  public:
   AutoDelete();
-  AutoDelete(Path p, bool recursive = true);
+  explicit AutoDelete(Path p, bool recursive = true);
   ~AutoDelete();
   void cancel();
   void reset(const Path& p, bool recursive = true);
-  operator Path() const { return path; }
+  explicit operator Path() const { return path; }
 };
 
 class AutoCloseFD {
@@ -180,7 +180,7 @@ class AutoCloseFD {
 
  public:
   AutoCloseFD();
-  AutoCloseFD(int fd);
+  explicit AutoCloseFD(int fd);
   AutoCloseFD(const AutoCloseFD& fd) = delete;
   AutoCloseFD(AutoCloseFD&& that);
   ~AutoCloseFD();
@@ -210,16 +210,24 @@ class Pid {
 
  public:
   Pid();
-  Pid(pid_t pid);
+  explicit Pid(pid_t pid);
   ~Pid();
   void operator=(pid_t pid);
-  operator pid_t();
+  explicit operator pid_t();
   int kill();
   int wait();
 
   void setSeparatePG(bool separatePG);
   void setKillSignal(int signal);
   pid_t release();
+
+  friend bool operator==(const Pid& lhs, const Pid& rhs) {
+    return lhs.pid == rhs.pid;
+  }
+
+  friend bool operator!=(const Pid& lhs, const Pid& rhs) {
+    return !(lhs == rhs);
+  }
 };
 
 /* Kill all processes running under the specified uid by sending them
@@ -275,7 +283,8 @@ class ExecError : public Error {
   int status;
 
   template <typename... Args>
-  ExecError(int status, Args... args) : Error(args...), status(status) {}
+  explicit ExecError(int status, Args... args)
+      : Error(args...), status(status) {}
 };
 
 /* Convert a list of strings to a null-terminated vector of char
@@ -378,7 +387,7 @@ class Callback {
   std::atomic_flag done = ATOMIC_FLAG_INIT;
 
  public:
-  Callback(std::function<void(std::future<T>)> fun) : fun(fun) {}
+  explicit Callback(std::function<void(std::future<T>)> fun) : fun(fun) {}
 
   Callback(Callback&& callback) : fun(std::move(callback.fun)) {
     auto prev = callback.done.test_and_set();
@@ -449,7 +458,8 @@ template <typename T>
 struct MaintainCount {
   T& counter;
   long delta;
-  MaintainCount(T& counter, long delta = 1) : counter(counter), delta(delta) {
+  explicit MaintainCount(T& counter, long delta = 1)
+      : counter(counter), delta(delta) {
     counter += delta;
   }
   ~MaintainCount() { counter -= delta; }