[PATCH] D106994: [modules] Fix miscompilation when using two RecordDecl definitions with the same name.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.


================
Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:3346-3347
 
+  if (auto *RD = dyn_cast<RecordDecl>(DC))
+    return RD->getDefinition();
+
----------------
In D71734 we have

```lang=c++
  if (auto *RD = dyn_cast<RecordDecl>(DC))
    if (!RD->getASTContext().getLangOpts().CPlusPlus)
      return RD->getCanonicalDecl()->getDefinition();
```

I've verified that in C++ unions are also CXXRecordDecl, so I think CPlusPlus check is not required. Locally I'm testing with

```lang=c++
  if (auto *RD = dyn_cast<RecordDecl>(DC)) {
    assert(!RD->getASTContext().getLangOpts().CPlusPlus &&
           "Unexpected RecordDecl in C++");
    return RD->getDefinition();
  }
```
to get extra reassurance. But don't think it should be in the final version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106994