mpark wrote:

> Now it looks much better.

@ChuanqiXu9 One more tweak here... `field_empty()` and family loads fields, 
which messes with some other assumptions and triggers an assertion failure. The 
PR now introduces a `noload_field_empty()` and family and uses that instead, 
which addresses the issue. I think it's also "what we mean". That is, we want 
to check that we have some fields currently without loading further.

I also tried just not checking the `field_empty()` condition, and relying on 
`numberAnonymousDeclsWithin` to be a no-op on an empty class. The problem here 
is that `numberAnonymousDeclsWithin` currently calls `decls()`, which triggers 
loading and triggers the same issue. Changing this to `noload_decls()` seems to 
do the trick, but it's not obvious that the usage of it in 
[ASTWriter.cpp](https://github.com/llvm/llvm-project/blob/e3c7b7f806559a361d2cf8374d65230c75bb5829/clang/lib/Serialization/ASTWriter.cpp#L7031)
 requires `decls()` semantics or not.

I'm proposing to go with the `noload_field_empty()` approach here.

https://github.com/llvm/llvm-project/pull/155948
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to