[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

================
@@ -6659,13 +6655,22 @@ void ASTWriter::MacroRead(serialization::MacroID ID, MacroInfo *MI) {
 }
 
 void ASTWriter::TypeRead(TypeIdx Idx, QualType T) {
-  // Always take the highest-numbered type index. This copes with an interesting
+  // Always take the type index that comes in later module files.
+  // This copes with an interesting
   // case for chained AST writing where we schedule writing the type and then,
   // later, deserialize the type from another AST. In this case, we want to
-  // keep the higher-numbered entry so that we can properly write it out to
+  // keep the just writing entry so that we can properly write it out to
   // the AST file.
   TypeIdx &StoredIdx = TypeIdxs[T];
-  if (Idx.getIndex() >= StoredIdx.getIndex())
+
+  // Ignore it if the type comes from the current being written module file.
----------------
ChuanqiXu9 wrote:

Since the current being written module file is the latest module. The reason why we don't need it previously is that, 
previously the type ID of the current being written module file is always larger than types from other module files.

For example, if A import B and B has 30 types, then previously, the type ID for the first type in A will be 31 + `NUM_PREDEFINED_TYPES`. This is also one of the motivation of the series patch. We don't hope the encoding of the current module file being affected by imported modules as much as possible.

So now, for the above example, the first type ID in A will be `<0, 1 + NUM_PREDEFINED_TYPES>` and type ID in B may be `<1, [0, 30]>`. So we can't compare them easily.

>  Do we have tests for it?

Yes, if we remove this line, there are failing tests.

https://github.com/llvm/llvm-project/pull/92511