[PATCH] D142384: [C++20] Fix a crash with modules.

[Ilya Biryukov via Phabricator via cfe-commits]

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