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

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

> > > @ilya-biryukov, could we have another try of this PR on your end?
> > 
> > 
> > Sorry for missing this, we will rerun the testing and get back to you.
> 
> In the meantime, could you rebase the patch on top of current ToT?

Are you saying to rebase on your local branch? Since there is no conflicts in the trunk.



> I'm getting this error when trying to bootstrap Clang:
> 
> ```
> In file included from clang/lib/Serialization/ASTReaderDecl.cpp:15:
> clang/lib/Serialization/ASTReaderInternals.h:160:19: error: ISO C++20 considers use of overloaded operator '==' (with operand types 'clang::serialization::reader::LazySpecializationInfo' and 'LazySpecializationInfo') to be ambiguous despite there being a unique best viable function [-Werror,-Wambiguous-reversed-operator]
>   160 |             if (I == Info)
>       |                 ~ ^  ~~~~
> clang/lib/Serialization/ASTReaderInternals.h:128:8: note: ambiguity is between a regular call to this operator and a call with the argument order reversed
>   128 |   bool operator==(const LazySpecializationInfo &Other) {
>       |        ^
> clang/lib/Serialization/ASTReaderInternals.h:128:8: note: mark 'operator==' as const or add a matching 'operator!=' to resolve the ambiguity
> ```
> 
> I can mute it for testing, but this also needs to be fixed.

Oh, I know this pattern. This is potential breaking change in C++20. I didn't catch this since LLVM is C++17 based. Let's mute the warning in the test.

> clang/test/Modules/odr_hash.cpp fails with
> 
> ```
> error: 'expected-error' diagnostics seen but not expected: 
>   File /tmp/odr_hash.cpp/odr_hash.cpp.tmp/Inputs/second.h Line 5028: 'DeclTemplateArguments::bar' has different definitions in different modules; definition in module 'SecondModule' first difference is function body
> error: 'expected-note' diagnostics seen but not expected: 
>   File /tmp/odr_hash.cpp/odr_hash.cpp.tmp/Inputs/first.h Line 4144: but in 'FirstModule' found a different body
> 2 errors generated.
> ```
> 
> coming from this invocation:
> 
> ```
> [ 28] // RUN: %clang_cc1 -triple x86_64-linux-gnu -x c++ -std=c++20 \
> [ 29] // RUN:   -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache \
> [ 30] // RUN:   -I%t/Inputs -verify %s
> ```

I've seen this issue in the past. It was about the order mismatch in the diagnostics since this patch changes the order to load specializations. But in the latest version, the mismatch disappeared. So it might be fine if the error you're seeing is only about the mismatches.

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