[PATCH] D154324: [C++20] [Modules] [ODRHash] Use CanonicalType for base classes

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In D154324#4520096 <https://reviews.llvm.org/D154324#4520096>, @alexfh wrote:

> In D154324#4516964 <https://reviews.llvm.org/D154324#4516964>, @alexfh wrote:
>
>> In D154324#4516917 <https://reviews.llvm.org/D154324#4516917>, @ChuanqiXu wrote:
>>
>>> Maybe we got something wrong with this. I'd like to revert this patch in case it breaks something. But would you like to reduce your reproducer further to a state without external includes to STL or protobuf? Then we can add the reduced reproducer to the tests to avoid further regressions.
>>
>> That turned out to be quite time-consuming, but I can try nevertheless. I also asked @rsmith if he could figure out what the problem is. Hopefully, he can help with the test case, if gets to the bottom of the problem.
>
> I have a reduced reproducer, but I still depends on the internal build setup. I need a bit more time to make the reproducer standalone.

Okay, here's the repro:
F28380583: modules-repro.tar.gz <https://reviews.llvm.org/F28380583>

And my observations with it:

  $ cat a.cppmap
  module "a" {
    export *
    module "a.h" {
      export *
      header "a.h"
    }
    use "c"
  }
  $ cat b.cppmap
  module "b" {
    export *
    module "b.h" {
      export *
      header "b.h"
    }
    use "c"
  }
  $ cat c.cppmap
  module "c" {
    export *
    module "c1.h" {
      export *
      textual header "c1.h"
    }
    module "c2.h" {
      export *
      textual header "c2.h"
    }
    module "c3.h" {
      export *
      textual header "c3.h"
    }
  }
  $ cat test.cppmap
  module "test" {
    export *
    use "a"
    use "b"
  }
  $ cat a.h
  #ifndef A_H_
  #define A_H_
  
  #include "c1.h"
  
  namespace q {
  template <typename T,
            typename std::enable_if<::p::P<T>::value>::type>
  class X {};
  }  // namespace q
  
  #include "c3.h"
  
  #endif  // A_H_
  $ cat b.h
  #ifndef B_H_
  #define B_H_
  
  #include "c2.h"
  
  #endif  // B_H_
  $ cat c1.h
  #ifndef C1_H_
  #define C1_H_
  
  namespace std {
  template <class _Tp, _Tp __v>
  struct integral_constant {
    static constexpr const _Tp value = __v;
    typedef _Tp value_type;
    typedef integral_constant type;
    constexpr operator value_type() const noexcept { return value; }
    constexpr value_type operator()() const noexcept { return value; }
  };
  
  template <class _Tp, _Tp __v>
  constexpr const _Tp integral_constant<_Tp, __v>::value;
  
  typedef integral_constant<bool, true> true_type;
  typedef integral_constant<bool, false> false_type;
  
  template <bool, class _Tp = void>
  struct enable_if {};
  template <class _Tp>
  struct enable_if<true, _Tp> {
    typedef _Tp type;
  };
  }  // namespace std
  
  namespace p {
  template <typename T>
  struct P : ::std::false_type {};
  }
  
  #endif  // C1_H_
  $ cat c2.h
  #ifndef C2_H_
  #define C2_H_
  
  #include "c3.h"
  
  enum E {};
  namespace p {
  template <>
  struct P<E> : std::true_type {};
  }  // namespace proto2
  
  inline void f(::util::EnumErrorSpace<E>) {}
  
  #endif  // C2_H_
  $ cat c3.h
  #ifndef C3_H_
  #define C3_H_
  
  #include "c1.h"
  
  namespace util {
  
  template <typename T>
  class ErrorSpaceImpl;
  
  class ErrorSpace {
   protected:
    template <bool* addr>
    struct OdrUse {
      constexpr OdrUse() : b(*addr) {}
      bool& b;
    };
    template <typename T>
    struct Registerer {
      static bool register_token;
      static constexpr OdrUse<&register_token> kRegisterTokenUse{};
    };
  
   private:
    template <typename T>
    static const ErrorSpace* GetBase() {
      return 0;
    }
  
    static bool Register(const ErrorSpace* (*space)()) { return true; }
  };
  
  template <typename T>
  bool ErrorSpace::Registerer<T>::register_token =
      Register(&ErrorSpace::GetBase<T>);
  
  template <typename T>
  class ErrorSpaceImpl : public ErrorSpace {
   private:
    static constexpr Registerer<ErrorSpaceImpl> kRegisterer{};
  };
  
  template <typename T, typename = typename std::enable_if<p::P<T>::value>::type>
  class EnumErrorSpace : public ErrorSpaceImpl<EnumErrorSpace<T>> {};
  
  }  // namespace util
  #endif  // C3_H_
  $ cat test.cc
  #include "a.h"
  #include "b.h"
  
  int main(int, char**) {}
  $ clang -fmodules -fno-implicit-modules -fno-implicit-module-maps -fmodule-name=c -fmodule-map-file=c.cppmap -xc++ -c c.cppmap -Xclang=-emit-module -o c.pcm
  $ clang -fmodules -fno-implicit-modules -fno-implicit-module-maps -fmodule-name=a -fmodule-map-file=a.cppmap -fmodule-map-file=c.cppmap -xc++ -c a.cppmap -Xclang=-emit-module -o a.pcm
  $ clang -fmodules -fno-implicit-modules -fno-implicit-module-maps -fmodule-name=b -fmodule-map-file=b.cppmap -fmodule-map-file=c.cppmap -xc++ -c b.cppmap -Xclang=-emit-module -o b.pcm
  $ clang -fmodules -fno-implicit-modules -fno-implicit-module-maps -fmodule-name=test -fmodule-map-file=test.cppmap -fmodule-map-file=a.cppmap -fmodule-map-file=b.cppmap -Xclang=-fmodule-file=a.pcm -Xclang=-fmodule-file=b.pcm -xc++ -c test.cc -o test.pcm
  In module 'b':
  ./c3.h:44:7: error: 'util::EnumErrorSpace' has different definitions in different modules; definition in module 'b.b.h' is here
     44 | class EnumErrorSpace : public ErrorSpaceImpl<EnumErrorSpace<T>> {};
        |       ^
  ./c3.h:44:7: note: definition in module 'a.a.h' is here
     44 | class EnumErrorSpace : public ErrorSpaceImpl<EnumErrorSpace<T>> {};
        |       ^
  1 error generated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154324/new/

https://reviews.llvm.org/D154324