[libcxx-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

Fri Jan 13 05:53:09 PST 2023

aaron.ballman added a comment.

In D131858#4050112 <https://reviews.llvm.org/D131858#4050112>, @rsmith wrote:

> In D131858#3957630 <https://reviews.llvm.org/D131858#3957630>, @arphaman wrote:
>
>> This change has caused a failure in Clang's stage 2 CI on the green dragon Darwin CI: https://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/6390/console.
>>
>>   Assertion failed: (lvaluePath->getType() == elemTy && "Unexpected type reference!"), function readAPValue, file /Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/tools/clang/include/clang/AST/AbstractBasicReader.inc, line 736.
>
> This assert is simply wrong, and I've removed it in rG2009f2450532450a99c1a03d5e2c30f478121839 <https://reviews.llvm.org/rG2009f2450532450a99c1a03d5e2c30f478121839> -- that change should be safe to cherry-pick into the release branch. It's possible for the recomputation of the type after deserialization to result in a different type than what we saw when serializing, because redeclarations of the same entity can use the same type with different sugar -- or even slightly different types in some cases, such as when an array bound is added in a redeclaration. The dumps of the types provided by @steven_wu confirms that we were just seeing a difference in type sugar in this case.
>
>>   Assertion failed: (BlockScope.empty() && CurAbbrevs.empty() && "Block imbalance"), function ~BitstreamWriter, file /Users/buildslave/jenkins/workspace/clang-stage1-RA/llvm-project/llvm/include/llvm/Bitstream/BitstreamWriter.h, line 119.
>
> Is this still happening? If so, this looks more serious, and will need further investigation.
>
> Can we undo the workaround in https://reviews.llvm.org/D139956 and see if the bot is now happy? Or can someone who was seeing problems before (@steven_wu?) run a test?

Thank you for poking at this Richard! However, I think we still need to revert the functionality in this area unless we're able to make headway on https://github.com/llvm/llvm-project/issues/59271 and quickly. FWIW, I ran into this exact problem yesterday on my dev machine, so the overhead is still a present concern. If that's something you plan to work on, then I think it'd make sense for Erich to hold off on starting the revert work to give you a chance to improve this. But if nobody is actively working on it, we need to start pulling this back because the branch date is a bit over a week away (Jan 24).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858