Merge pull request #6258 from obsidiansystems/gcc-bug-ergonomics

Remove bug-avoiding `StoreConfig *` casts for settings

(cherry picked from commit e3febfcd532adb23ca05ac465a2b907d6f1a3529)
Change-Id: Ifeae276582fdbc781a38581df9de3da67a7e7bf9

configure.ac

@@ -68,6 +68,9 @@ case "$host_os" in
 esac
 
 
+ENSURE_NO_GCC_BUG_80431
+
+
 # Check for pubsetbuf.
 AC_MSG_CHECKING([for pubsetbuf])
 AC_LANG_PUSH(C++)

m4/gcc_bug_80431.m4

@@ -0,0 +1,64 @@
+# Ensure that this bug is not present in the C++ toolchain we are using.
+#
+# URL for bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80431
+#
+# The test program is from that issue, with only a slight modification
+# to set an exit status instead of printing strings.
+AC_DEFUN([ENSURE_NO_GCC_BUG_80431],
+[
+  AC_MSG_CHECKING([that GCC bug 80431 is fixed])
+  AC_LANG_PUSH(C++)
+  AC_RUN_IFELSE(
+    [AC_LANG_PROGRAM(
+      [[
+        #include <cstdio>
+
+        static bool a = true;
+        static bool b = true;
+
+        struct Options { };
+
+        struct Option
+        {
+            Option(Options * options)
+            {
+                a = false;
+            }
+
+            ~Option()
+            {
+                b = false;
+            }
+        };
+
+        struct MyOptions : Options { };
+
+        struct MyOptions2 : virtual MyOptions
+        {
+            Option foo{this};
+        };
+      ]],
+      [[
+        {
+            MyOptions2 opts;
+        }
+        return (a << 1) | b;
+      ]])],
+    [status_80431=0],
+    [status_80431=$?],
+    [
+      # Assume we're bug-free when cross-compiling
+    ])
+  AC_LANG_POP(C++)
+  AS_CASE([$status_80431],
+    [0],[
+      AC_MSG_RESULT(yes)
+    ],
+    [2],[
+      AC_MSG_RESULT(no)
+      AC_MSG_ERROR(Cannot build Nix with C++ compiler with this bug)
+    ],
+    [
+      AC_MSG_RESULT(unexpected result $status_80431: not expected failure with bug, ignoring)
+    ])
+])

src/libstore/binary-cache-store.hh

@@ -17,28 +17,28 @@ struct BinaryCacheStoreConfig : virtual StoreConfig
 {
     using StoreConfig::StoreConfig;
 
-    const Setting<std::string> compression{(StoreConfig*) this, "xz", "compression",
+    const Setting<std::string> compression{this, "xz", "compression",
         "NAR compression method (`xz`, `bzip2`, `gzip`, `zstd`, or `none`)."};
 
-    const Setting<bool> writeNARListing{(StoreConfig*) this, false, "write-nar-listing",
+    const Setting<bool> writeNARListing{this, false, "write-nar-listing",
         "Whether to write a JSON file that lists the files in each NAR."};
 
-    const Setting<bool> writeDebugInfo{(StoreConfig*) this, false, "index-debug-info",
+    const Setting<bool> writeDebugInfo{this, false, "index-debug-info",
         R"(
           Whether to index DWARF debug info files by build ID. This allows [`dwarffs`](https://github.com/edolstra/dwarffs) to
           fetch debug info on demand
         )"};
 
-    const Setting<Path> secretKeyFile{(StoreConfig*) this, "", "secret-key",
+    const Setting<Path> secretKeyFile{this, "", "secret-key",
         "Path to the secret key used to sign the binary cache."};
 
-    const Setting<Path> localNarCache{(StoreConfig*) this, "", "local-nar-cache",
+    const Setting<Path> localNarCache{this, "", "local-nar-cache",
         "Path to a local cache of NARs fetched from this binary cache, used by commands such as `nix store cat`."};
 
-    const Setting<bool> parallelCompression{(StoreConfig*) this, false, "parallel-compression",
+    const Setting<bool> parallelCompression{this, false, "parallel-compression",
         "Enable multi-threaded compression of NARs. This is currently only available for `xz` and `zstd`."};
 
-    const Setting<int> compressionLevel{(StoreConfig*) this, -1, "compression-level",
+    const Setting<int> compressionLevel{this, -1, "compression-level",
         R"(
           The *preset level* to be used when compressing NARs.
           The meaning and accepted values depend on the compression method selected.

src/libstore/legacy-ssh-store.cc

@@ -17,10 +17,10 @@ struct LegacySSHStoreConfig : virtual CommonSSHStoreConfig
 {
     using CommonSSHStoreConfig::CommonSSHStoreConfig;
 
-    const Setting<Path> remoteProgram{(StoreConfig*) this, "nix-store", "remote-program",
+    const Setting<Path> remoteProgram{this, "nix-store", "remote-program",
         "Path to the `nix-store` executable on the remote machine."};
 
-    const Setting<int> maxConnections{(StoreConfig*) this, 1, "max-connections",
+    const Setting<int> maxConnections{this, 1, "max-connections",
         "Maximum number of concurrent SSH connections."};
 
     const std::string name() override { return "SSH Store"; }
@@ -38,7 +38,7 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
     // Hack for getting remote build log output.
     // Intentionally not in `LegacySSHStoreConfig` so that it doesn't appear in
     // the documentation
-    const Setting<int> logFD{(StoreConfig*) this, -1, "log-fd", "file descriptor to which SSH's stderr is connected"};
+    const Setting<int> logFD{this, -1, "log-fd", "file descriptor to which SSH's stderr is connected"};
 
     struct Connection
     {

src/libstore/local-fs-store.hh

@@ -11,25 +11,21 @@ struct LocalFSStoreConfig : virtual StoreConfig
 {
     using StoreConfig::StoreConfig;
 
-    // FIXME: the (StoreConfig*) cast works around a bug in gcc that causes
+    const OptionalPathSetting rootDir{this, std::nullopt,
-    // it to omit the call to the Setting constructor. Clang works fine
-    // either way.
-
-    const OptionalPathSetting rootDir{(StoreConfig*) this, std::nullopt,
         "root",
         "Directory prefixed to all other paths."};
 
-    const PathSetting stateDir{(StoreConfig*) this,
+    const PathSetting stateDir{this,
         rootDir.get() ? *rootDir.get() + "/nix/var/nix" : settings.nixStateDir,
         "state",
         "Directory where Nix will store state."};
 
-    const PathSetting logDir{(StoreConfig*) this,
+    const PathSetting logDir{this,
         rootDir.get() ? *rootDir.get() + "/nix/var/log/nix" : settings.nixLogDir,
         "log",
         "directory where Nix will store log files."};
 
-    const PathSetting realStoreDir{(StoreConfig*) this,
+    const PathSetting realStoreDir{this,
         rootDir.get() ? *rootDir.get() + "/nix/store" : storeDir, "real",
         "Physical path of the Nix store."};
 };

src/libstore/local-store.hh

@@ -40,12 +40,12 @@ struct LocalStoreConfig : virtual LocalFSStoreConfig
 {
     using LocalFSStoreConfig::LocalFSStoreConfig;
 
-    Setting<bool> requireSigs{(StoreConfig*) this,
+    Setting<bool> requireSigs{this,
         settings.requireSigs,
         "require-sigs",
         "Whether store paths copied into this store should have a trusted signature."};
 
-    Setting<bool> readOnly{(StoreConfig*) this,
+    Setting<bool> readOnly{this,
         false,
         "read-only",
         R"(

src/libstore/remote-store.hh

@@ -22,10 +22,10 @@ struct RemoteStoreConfig : virtual StoreConfig
 {
     using StoreConfig::StoreConfig;
 
-    const Setting<int> maxConnections{(StoreConfig*) this, 1, "max-connections",
+    const Setting<int> maxConnections{this, 1, "max-connections",
         "Maximum number of concurrent connections to the Nix daemon."};
 
-    const Setting<unsigned int> maxConnectionAge{(StoreConfig*) this,
+    const Setting<unsigned int> maxConnectionAge{this,
         std::numeric_limits<unsigned int>::max(),
         "max-connection-age",
         "Maximum age of a connection before it is closed."};

src/libstore/s3-binary-cache-store.cc

@@ -193,20 +193,20 @@ struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
 {
     using BinaryCacheStoreConfig::BinaryCacheStoreConfig;
 
-    const Setting<std::string> profile{(StoreConfig*) this, "", "profile",
+    const Setting<std::string> profile{this, "", "profile",
         R"(
           The name of the AWS configuration profile to use. By default
           Nix will use the `default` profile.
         )"};
 
-    const Setting<std::string> region{(StoreConfig*) this, Aws::Region::US_EAST_1, "region",
+    const Setting<std::string> region{this, Aws::Region::US_EAST_1, "region",
         R"(
           The region of the S3 bucket. If your bucket is not in
           `us–east-1`, you should always explicitly specify the region
           parameter.
         )"};
 
-    const Setting<std::string> scheme{(StoreConfig*) this, "", "scheme",
+    const Setting<std::string> scheme{this, "", "scheme",
         R"(
           The scheme used for S3 requests, `https` (default) or `http`. This
           option allows you to disable HTTPS for binary caches which don't
@@ -218,7 +218,7 @@ struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
           > information.
         )"};
 
-    const Setting<std::string> endpoint{(StoreConfig*) this, "", "endpoint",
+    const Setting<std::string> endpoint{this, "", "endpoint",
         R"(
           The URL of the endpoint of an S3-compatible service such as MinIO.
           Do not specify this setting if you're using Amazon S3.
@@ -229,13 +229,13 @@ struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
           > addressing instead of virtual host based addressing.
         )"};
 
-    const Setting<std::string> narinfoCompression{(StoreConfig*) this, "", "narinfo-compression",
+    const Setting<std::string> narinfoCompression{this, "", "narinfo-compression",
         "Compression method for `.narinfo` files."};
 
-    const Setting<std::string> lsCompression{(StoreConfig*) this, "", "ls-compression",
+    const Setting<std::string> lsCompression{this, "", "ls-compression",
         "Compression method for `.ls` files."};
 
-    const Setting<std::string> logCompression{(StoreConfig*) this, "", "log-compression",
+    const Setting<std::string> logCompression{this, "", "log-compression",
         R"(
           Compression method for `log/*` files. It is recommended to
           use a compression method supported by most web browsers
@@ -243,11 +243,11 @@ struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
         )"};
 
     const Setting<bool> multipartUpload{
-        (StoreConfig*) this, false, "multipart-upload",
+        this, false, "multipart-upload",
         "Whether to use multi-part uploads."};
 
     const Setting<uint64_t> bufferSize{
-        (StoreConfig*) this, 5 * 1024 * 1024, "buffer-size",
+        this, 5 * 1024 * 1024, "buffer-size",
         "Size (in bytes) of each part in multi-part uploads."};
 
     const std::string name() override { return "S3 Binary Cache Store"; }

src/libstore/ssh-store-config.hh

@@ -9,16 +9,16 @@ struct CommonSSHStoreConfig : virtual StoreConfig
 {
     using StoreConfig::StoreConfig;
 
-    const Setting<Path> sshKey{(StoreConfig*) this, "", "ssh-key",
+    const Setting<Path> sshKey{this, "", "ssh-key",
         "Path to the SSH private key used to authenticate to the remote machine."};
 
-    const Setting<std::string> sshPublicHostKey{(StoreConfig*) this, "", "base64-ssh-public-host-key",
+    const Setting<std::string> sshPublicHostKey{this, "", "base64-ssh-public-host-key",
         "The public host key of the remote machine."};
 
-    const Setting<bool> compress{(StoreConfig*) this, false, "compress",
+    const Setting<bool> compress{this, false, "compress",
         "Whether to enable SSH compression."};
 
-    const Setting<std::string> remoteStore{(StoreConfig*) this, "", "remote-store",
+    const Setting<std::string> remoteStore{this, "", "remote-store",
         R"(
           [Store URL](@docroot@/command-ref/new-cli/nix3-help-stores.md#store-url-format)
           to be used on the remote machine. The default is `auto`

src/libstore/ssh-store.cc

@@ -16,7 +16,7 @@ struct SSHStoreConfig : virtual RemoteStoreConfig, virtual CommonSSHStoreConfig
     using RemoteStoreConfig::RemoteStoreConfig;
     using CommonSSHStoreConfig::CommonSSHStoreConfig;
 
-    const Setting<Path> remoteProgram{(StoreConfig*) this, "nix-daemon", "remote-program",
+    const Setting<Path> remoteProgram{this, "nix-daemon", "remote-program",
         "Path to the `nix-daemon` executable on the remote machine."};
 
     const std::string name() override { return "Experimental SSH Store"; }