[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

> > what the code does is: when we write a on-disk hash table, try to write the imported merged hash table in the same process so that we don't need to read these tables again. However, in line 329 the function will try to omit the data from imported table with the same key which already emitted by the current module file. This is the root cause of the problem.
> 
> It's been a while since I looked at this, but as I recall, a fundamental assumption of MultiOnDiskHashTable is that if we have a lookup result for a key K in the current file, that result supersedes any results from dependency files. So lookup won't look in those files if we have a local result (they are overridden) and merging doesn't take results from those files either.
> 
> So I think the problem probably is that when we form a local result, we need to (but presumably don't) add all the imported results with the same key to the local result.

I took a second look at MultiOnDiskHashTable.h and it looks like it is what I did. There is not any logic to supersedes results from dependency files in MultiOnDiskHashTable itself (or it doesn't have the concept of dependency file in MultiOnDiskHashTable.) The only similar thing is `MultiOnDiskHashTableGenerator::emit`, where the contents from the merged tables of imported files will be come the contents of the current file. And the corresponding imported files will be marked as overriden and removed in `find`.

And in this patch, I've already tried to write all the contents into the current one. So I feel the concern is already addressed.

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