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

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In D154324#4516917 <https://reviews.llvm.org/D154324#4516917>, @ChuanqiXu wrote:

> 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.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154324