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

[Chuanqi Xu via llvm-branch-commits]

================
@@ -7392,27 +7388,28 @@ QualType ASTReader::GetType(TypeID ID) {
   return TypesLoaded[Index].withFastQualifiers(FastQuals);
 }
 
-QualType ASTReader::getLocalType(ModuleFile &F, unsigned LocalID) {
+QualType ASTReader::getLocalType(ModuleFile &F, TypeID LocalID) {
----------------
ChuanqiXu9 wrote:

> Could we clarify what LocalID means here in a comment somewhere?

In ASTReader.h, there is a single line comment: `Resolve a local type ID within a given AST file into a type.`. I add a new line to explain it as "A local type ID is only meaningful with the corresponding module file. See the implementation of getGlobalTypeID for details.".

> IIUC, the ModuleFileIndex there is an index into F.TransitiveImports rather than into a "global" module manager.

Yes, this is correct for local ID. But I feel it might be odd to put this in the comment. I feel, it might be good to know, we can use `ASTReader::getGlobalTypeID` to convert a `<ModuleFile, LocalTypeID>` pair to a `GlobalTypeID`. And the concrete process is the implementation details to GlobalTypeID`.

> Previously, we had distinction between unsigned in parameters to this function and TypeID as a return type.
Maybe we should keep this here and accept uint64_t to avoid confusing the common global module file index and a local file index?

I don't feel it better. `uint64_t` doesn't provide any type information. To be best, maybe we can do something like for `LocalDeclID` and `GlobalDeclID`. I only did that for DeclID since I feel the DeclID is the most complicated and the TypeID are much more simpler. I think we can this later if we feel it is necessary.

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