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

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

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

> Hi, we've started seeing compilation errors with our modularized build after this commit. The errors say `'SomeType' has different definitions in different modules`, but then point to the same definition that comes from the same textual header included into two modules.
>
> The setup (which I couldn't completely isolate yet) is roughly similar to this (hopefully, I didn't miss any important parts):
>
> Textual header p.h:
>
>   #include <type_traits>
>   
>   #include "protobuf/generated_enum_util.h"
>   ...
>   
>   template <typename T,
>             typename =
>                 typename std::enable_if<proto2::is_proto_enum<T>::value>::type>
>   class SomeType : E<S<T>> {
>   ...
>   };
>
> Module A, a.h:
>
>   #include <type_traits>
>   
>   #include "protobuf/generated_enum_util.h"
>   
>   namespace q {
>   template <typename T,
>             typename std::enable_if<::proto2::is_proto_enum<T>::value>::type>
>   class X {};
>   }
>   
>   #include "p.h"
>
> Module B, b.h:
>
>   // ...
>   // something likely unrelated
>   // ...
>   #include "p.h"
>
> Module C (uses module A, module B), c.h:
>
>   #include "a.h"
>   #include "b.h"

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154324