abseil / abseil-cpp

Abseil Common Libraries (C++)
https://abseil.io
Apache License 2.0
15k stars 2.62k forks source link

[Bug]: Compilation regression in version update: "...incomplete type 'absl::container_internal::HashEq..." #1745

Open monkeynova opened 2 months ago

monkeynova commented 2 months ago

Describe the issue

I updated from 20240116.2 to 20240722.0 and had a compilation regression of previous code. There was some nontrivial template meta-programming involved, but the part that failed for me was around returning an absl::flat_hash_set from a method in a templated type Foo.

This commit documents my upgrade and the change I had to make, though it's a private repository, so I'm not sure how best to share that with you.

I stripped the problem down to a bare reproduction step that can exist as a super-simple bazel repository which I've included in reproduction steps. Bringing the version back in MODULES.bazel allows compilation. As does setting the #define I've included in the code which documents my current workaround for this regression.

The error messages I see are as follows

INFO: Analyzed target //:im_replicate_test (0 packages loaded, 0 targets configured).
ERROR: /Users/keith/github/im_replicate/BUILD:1:8: Compiling im_replicate_test.cc failed: (Exit 1): cc_wrapper.sh failed: error executing CppCompile command (from target //:im_replicate_test) external/bazel_tools~cc_configure_extension~local_config_cc/cc_wrapper.sh -U_FORTIFY_SOURCE -fstack-protector -Wall -Wthread-safety -Wself-assign -Wunused-but-set-parameter -Wno-free-nonheap-object ... (remaining 50 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
In file included from im_replicate_test.cc:3:
In file included from external/abseil-cpp~/absl/container/flat_hash_map.h:43:
In file included from external/abseil-cpp~/absl/container/hash_container_defaults.h:19:
external/abseil-cpp~/absl/container/internal/hash_function_defaults.h:263:1: error: incomplete type 'absl::container_internal::HashEq<IntegerMod<long long, StaticModTraits<long long, long long, 37>>>' named in nested name specifier
using hash_default_hash = typename container_internal::HashEq<T>::Hash;
^~~~~
external/abseil-cpp~/absl/container/hash_container_defaults.h:31:1: note: in instantiation of template type alias 'hash_default_hash' requested here
using DefaultHashContainerHash = absl::container_internal::hash_default_hash<T>;
^
external/abseil-cpp~/absl/container/flat_hash_set.h:122:33: note: in instantiation of template type alias 'DefaultHashContainerHash' requested here
template <class T, class Hash = DefaultHashContainerHash<T>,
                                ^
im_replicate_test.cc:48:9: note: in instantiation of default argument for 'flat_hash_set<IntegerMod<long long, StaticModTraits<long long, long long, 37>>>' required here
  absl::flat_hash_set<IntegerMod> Sqrt() const {
        ^~~~~~~~~~~~~~~~~~~~~~~~~
external/abseil-cpp~/absl/container/internal/hash_function_defaults.h:197:54: note: in instantiation of template class 'IntegerMod<long long, StaticModTraits<long long, long long, 37>>' requested here
struct HasAbslContainerHash<T, absl::void_t<typename T::absl_container_hash>>
                                                     ^
external/abseil-cpp~/absl/container/internal/hash_function_defaults.h:247:44: note: during template argument deduction for class template partial specialization 'HasAbslContainerHash<T, absl::void_t<typename T::absl_container_hash>>' [with T = IntegerMod<long long, StaticModTraits<long long, long long, 37>>]
struct HashEq<T, typename std::enable_if_t<HasAbslContainerHash<T>::value>> {
                                           ^
external/abseil-cpp~/absl/container/internal/hash_function_defaults.h:247:44: note: (skipping 1 context in backtrace; use -ftemplate-backtrace-limit=0 to see all)
external/abseil-cpp~/absl/container/internal/hash_function_defaults.h:263:1: note: during template argument deduction for class template partial specialization 'HashEq<T, typename std::enable_if_t<HasAbslContainerHash<T>::value>>' [with T = IntegerMod<long long, StaticModTraits<long long, long long, 37>>]
using hash_default_hash = typename container_internal::HashEq<T>::Hash;
^
external/abseil-cpp~/absl/container/internal/hash_function_defaults.h:263:1: note: in instantiation of template class 'absl::container_internal::HashEq<IntegerMod<long long, StaticModTraits<long long, long long, 37>>>' requested here
external/abseil-cpp~/absl/container/hash_container_defaults.h:31:1: note: in instantiation of template type alias 'hash_default_hash' requested here
using DefaultHashContainerHash = absl::container_internal::hash_default_hash<T>;
^
external/abseil-cpp~/absl/container/flat_hash_set.h:122:33: note: in instantiation of template type alias 'DefaultHashContainerHash' requested here
template <class T, class Hash = DefaultHashContainerHash<T>,
                                ^
im_replicate_test.cc:82:9: note: in instantiation of default argument for 'flat_hash_set<IntegerMod<long long, StaticModTraits<long long, long long, 37>>>' required here
  absl::flat_hash_set<ModType> set;
        ^~~~~~~~~~~~~~~~~~~~~~
external/abseil-cpp~/absl/container/internal/hash_function_defaults.h:71:8: note: forward declaration of 'absl::container_internal::HashEq<IntegerMod<long long, StaticModTraits<long long, long long, 37>>>'
struct HashEq {
       ^
1 error generated.
Target //:im_replicate_test failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 1.509s, Critical Path: 1.39s
INFO: 2 processes: 2 internal.
ERROR: Build did NOT complete successfully
//:im_replicate_test                                            FAILED TO BUILD

Executed 0 out of 1 test: 1 fails to build.

Steps to reproduce the problem

bazel test ...

With the following repository

MODULES.bazel

# works with 20240116.2
bazel_dep(name = "abseil-cpp", version = "20240722.0", repo_name = "com_google_absl")
bazel_dep(name = "googletest", version = "1.15.2", repo_name = "com_google_googletest")

BUILD

cc_test(
    name = "im_replicate_test",
    srcs = ["im_replicate_test.cc"],
    deps = [
        "@com_google_absl//absl/container:flat_hash_map",
        "@com_google_absl//absl/container:flat_hash_set",
        "@com_google_absl//absl/flags:parse",
        "@com_google_absl//absl/flags:usage",
        "@com_google_absl//absl/log:check",
        "@com_google_absl//absl/log:initialize",
        "@com_google_absl//absl/strings",
        "@com_google_googletest//:gtest",
    ],
)

im_replicate_test.cc

#include <iostream>

#include "absl/container/flat_hash_map.h"
#include "absl/container/flat_hash_set.h"
#include "absl/flags/parse.h"
#include "absl/flags/usage.h"
#include "absl/log/check.h"
#include "absl/log/initialize.h"
#include "absl/strings/str_join.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"

template <typename Storage, typename LiteralType, LiteralType kMod>
struct StaticModTraits {
  bool operator==(const StaticModTraits&) const = default;
  constexpr Storage mod() const { return kMod; }
};

template <typename Storage>
struct DynamicModTraits {
  DynamicModTraits(Storage mod) : mod_(mod) {}

  bool operator==(const DynamicModTraits&) const = default;

  Storage mod() const { return mod_; }
 private:
  Storage mod_;
};

template <typename Storage, typename Traits = DynamicModTraits<Storage>>
class IntegerMod {
 public:
  IntegerMod() = default;

  template <typename... TraitArgs>
  constexpr IntegerMod(const Storage& v, TraitArgs... trait_args)
      : traits_(trait_args...), v_(v % traits_.mod()) {}

  bool operator==(const IntegerMod& o) const = default;

#ifdef WORKING_VERSION
  // This version allows compilation.
  template <typename Containee=IntegerMod>
  absl::flat_hash_set<Containee> Sqrt() const {
    return {};
  }
#else
  absl::flat_hash_set<IntegerMod> Sqrt() const {
    return {};
  }
#endif

  template <typename H>
  friend H AbslHashValue(H h, const IntegerMod& m) {
    return H::combine(std::move(h), m.v_);
  }

  template <typename Sink>
  friend void AbslStringify(Sink& sink, const IntegerMod& m) {
    absl::Format(&sink, "(%v MOD %v)", m.v_, m.mod());
  }

  friend std::ostream& operator<<(std::ostream& out, const IntegerMod& m) {
    return out << absl::StreamFormat("%v", m);
  }

 private:
  Traits traits_;
  Storage v_ = 0;
};

template <typename Storage, Storage n>
using StaticIntegerMod =
    IntegerMod<Storage, StaticModTraits<Storage, Storage, n>>;

TEST(IntegerMod, Hash_Static) {
  // absl::flat_hash_set<StaticIntegerMod<...>> encountered template
  // instantiation problems (see comment on IntegerMod::Sqrt). This is here
  // to ensure that we hit those problems in euler/... and to help replicate
  // in a controlled environment.
  using ModType = StaticIntegerMod<int64_t, 37>;
  absl::flat_hash_set<ModType> set;
  set.insert(ModType{5});
  set.insert(ModType{7});
  EXPECT_EQ(set.size(), 2);
  set.insert(ModType{42});
  EXPECT_EQ(set.size(), 2);
}

std::vector<char*> InitMain(int argc, char** argv, const std::string& usage) {
  absl::SetProgramUsageMessage(usage);
  absl::InitializeLog();

  return absl::ParseCommandLine(argc, argv);
}

int main(int argc, char** argv) {
  std::vector<char*> args = InitMain(
    argc, argv,
    absl::StrCat("This program runs the collection of gunit tests (default) "
                 "or benchmarks linked in (if the --benchmark flag is set). "
                 "No arguments are accepted. Usage:\n", argv[0],
                 " [--benchmark]"));
  CHECK_EQ(args.size(), 1) << absl::StrJoin(args, ",");

  return RUN_ALL_TESTS();
}

What version of Abseil are you using?

20240116.2 (works) 20240722.0 (doesn't)

What operating system and version are you using?

macos 14.6.1

What compiler and version are you using?

default with the os. I think that's clang.

clang -v

Apple clang version 15.0.0 (clang-1500.3.9.4)
Target: arm64-apple-darwin23.6.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

What build system are you using?

bazel --version -> bazel 7.3.0 (via alas bazelisk=bazel)

Additional context

No response

monkeynova commented 2 months ago

With a little more digging, I don't need that level of templating to replicate the regression. The following does it too:

im_replicate_test.cc

#include <iostream>

#include "absl/container/flat_hash_map.h"
#include "absl/container/flat_hash_set.h"
#include "absl/flags/parse.h"
#include "absl/flags/usage.h"
#include "absl/log/check.h"
#include "absl/log/initialize.h"
#include "absl/strings/str_join.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"

template <typename Storage>
class IntegerMod {
 public:
  IntegerMod() = default;

  constexpr IntegerMod(const int64_t& v, int64_t mod)
      : mod_(mod), v_(v % mod)  {}

  bool operator==(const IntegerMod& o) const = default;

#ifdef WORKING_VERSION
  // This version allows compilation.
  template <typename Containee=IntegerMod>
  absl::flat_hash_set<Containee> Sqrt() const {
    return {};
  }
#else
  absl::flat_hash_set<IntegerMod> Sqrt() const {
    return {};
  }
#endif

  template <typename H>
  friend H AbslHashValue(H h, const IntegerMod& m) {
    return H::combine(std::move(h), m.v_);
  }

  template <typename Sink>
  friend void AbslStringify(Sink& sink, const IntegerMod& m) {
    absl::Format(&sink, "(%v MOD %v)", m.v_, m.mod_);
  }

  friend std::ostream& operator<<(std::ostream& out, const IntegerMod& m) {
    return out << absl::StreamFormat("%v", m);
  }

 private:
  int64_t mod_ = 1;
  int64_t v_ = 0;
};

TEST(IntegerMod, Hash_Static) {
  // absl::flat_hash_set<StaticIntegerMod<...>> encountered template
  // instantiation problems (see comment on IntegerMod::Sqrt). This is here
  // to ensure that we hit those problems in euler/... and to help replicate
  // in a controlled environment.
  using ModType = IntegerMod<int64_t>;
  absl::flat_hash_set<ModType> set;
  set.insert(ModType{5, 37});
  set.insert(ModType{7, 37});
  EXPECT_EQ(set.size(), 2);
  set.insert(ModType{42, 37});
  EXPECT_EQ(set.size(), 2) << absl::StrJoin(set, ",", [](std::string* out, ModType m) { 
    absl::StrAppend(out, m);
  });
}

std::vector<char*> InitMain(int argc, char** argv, const std::string& usage) {
  absl::SetProgramUsageMessage(usage);
  absl::InitializeLog();

  return absl::ParseCommandLine(argc, argv);
}

int main(int argc, char** argv) {
  std::vector<char*> args = InitMain(
    argc, argv,
    absl::StrCat("This program runs the collection of gunit tests (default) "
                 "or benchmarks linked in (if the --benchmark flag is set). "
                 "No arguments are accepted. Usage:\n", argv[0],
                 " [--benchmark]"));
  CHECK_EQ(args.size(), 1) << absl::StrJoin(args, ",");

  testing::InitGoogleTest(&argc, argv);
  return RUN_ALL_TESTS();
}
monkeynova commented 2 months ago

git bisect with this replication case points me to this commit which feels relevant as it looks like its a partial specialization of the HashEq struct which could have interfered with the ability to resolve the templating for my type

https://github.com/abseil/abseil-cpp/commit/643b48a3b4362a932c0e41afce62deb55adf825b