ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land.
In D142384#4098650 <https://reviews.llvm.org/D142384#4098650>, @usaxena95 wrote: > I think this is fine, and we should just use the definition when it is > available without asking the callers to request fields only when definition > is available. Not sure about C, but ObjC stacktrace definitely looks like a potential bug to me. We actually might introduce new failure modes (calling the function on the same `RecordDecl` without fields may start retuning different results). However, we also need to fix this crash in C++, so I suggest to land this and see if this will cause any fallout. ================ Comment at: clang/lib/AST/Decl.cpp:4772 LoadFieldsFromExternalStorage(); - + if (RecordDecl *D = getDefinition(); D && D != this) + return D->field_begin(); ---------------- Let's add a comment to make sure we avoid someone deleting this code. (Since we don't have a test). ``` // This is necessary for correctness for C++ with modules. // FIXME: come up with a test case that breaks. ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142384/new/ https://reviews.llvm.org/D142384 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
