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

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 added a comment.

In D142384#4096935 <https://reviews.llvm.org/D142384#4096935>, @ilya-biryukov wrote:

> @usaxena95 could you give an example of the code that fails the assertion? Is it some of the tests?

`assert(getDefinition())` fails about 3.6k tests.
`assert(!isa<CXXRecordDecl>(this) || getDefinition());` fails a handful 23 tests in ObjC:

  Failed Tests (23):
    Clang :: Analysis/CFContainers.mm
    Clang :: Analysis/blocks.m
    Clang :: Analysis/dtor-array.cpp
    Clang :: Analysis/edges-new.mm
    Clang :: Analysis/incorrect-checker-names.mm
    Clang :: Analysis/malloc.mm
    Clang :: Analysis/mig.mm
    Clang :: Analysis/retain-release.m
    Clang :: Analysis/retain-release.mm
    Clang :: Analysis/stack-capture-leak-arc.mm
    Clang :: Analysis/stack-capture-leak-no-arc.mm
    Clang :: Analysis/taint-tester.cpp
    Clang :: CodeGenObjCXX/encode.mm
    Clang-Unit :: AST/./ASTTests/0/100
    Clang-Unit :: AST/./ASTTests/1/100
    Clang-Unit :: AST/./ASTTests/14/100
    Clang-Unit :: AST/./ASTTests/16/100
    Clang-Unit :: AST/./ASTTests/76/100
    Clang-Unit :: AST/./ASTTests/77/100
    Clang-Unit :: AST/./ASTTests/78/100
    Clang-Unit :: AST/./ASTTests/79/100
    Clang-Unit :: AST/./ASTTests/98/100
    Clang-Unit :: AST/./ASTTests/99/100

`bin/clang  $SRC/llvm-project/clang/test/CodeGenObjCXX/encode.mm`
Stacktrace: https://pastebin.pl/view/c6f505a7

`assert((!isa<CXXRecordDecl>(this) || getDefinition() || !FirstDecl) && "Field without a CXX definition ?");` works. So if there is no CXX definition then the fields are always empty and current callers are fine with that.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142384