[AST] Fix an assertion violation in FieldDecl::getParent.

[hokein]

Summary

FieldDecl::getParent assumes that the FiledDecl::getDeclContext returns a
RecordDecl, this is true for C/C++, but not for ObjCIvarDecl:

The Decls hierarchy is like following

FieldDecl <-- ObjCIvarDecl

DeclContext <-- ObjCContainerDecl <-- ObjCInterfaceDecl

^
|----- TagDecl <-- RecordDecl

calling getParent() on ObjCIvarDecl will:

1. invoke getDeclContext(), which returns a DeclContext*, which points to an ObjCInterfaceDecl;
2. then downcast the "DeclContext" pointer to a RecordDecl*, and we will hit

the "is_a<RecordDecl>" assertion in llvm::cast (undefined behavior
in release build without assertion enabled);

[sammccall]

Sorry for stalling this. This is a sad situation, the API is actively misleading about the invariants.

Even the new API isn't very safe: getParent() isn't a great name if it only works on *some* things that have parents.
@rsmith any opinion on what we should do with this one? Would a rename to getParentRecord() help much? Something more invasive?

Approving since this is already a bit better and we can fix a crash.

clang-tools-extra/clangd/Hover.cpp

void addLayoutInfo(const NamedDecl &ND, HoverInfo &HI) {
 
   if (auto *RD = llvm::dyn_cast<RecordDecl>(&ND)) {
     if (auto Size = Ctx.getTypeSizeInCharsIfKnown(RD->getTypeForDecl()))
       HI.Size = Size->getQuantity();
     return;
   }
 
   if (const auto *FD = llvm::dyn_cast<FieldDecl>(&ND)) {
-    const auto *Record = FD->getParent()->getDefinition();
+    const auto *Record = FD->getParent();
+    if (Record)
+      Record = Record->getDefinition();
     if (Record && !Record->isDependentType()) {
       uint64_t OffsetBits = Ctx.getFieldOffset(FD);
       if (auto Size = Ctx.getTypeSizeInCharsIfKnown(FD->getType())) {
         HI.Size = Size->getQuantity();
         HI.Offset = OffsetBits / 8;
       }
     }
     return;

clang-tools-extra/clangd/unittests/HoverTests.cpp

const std::function<void(HoverInfo &)> ExpectedBuilder;
             HI.Kind = index::SymbolKind::Function;
             HI.Type = "void (const int &)";
             HI.ReturnType = "void";
             HI.Parameters = {
                 {std::string("const int &"), llvm::None, std::string("T()")}};
             HI.Definition = "template <> void foo<int>(const int &)";
             HI.NamespaceScope = "";
           }},
+      {
+          R"cpp(// should not crash
+           @interface ObjC {
+             char [[da^ta]];
+           }@end
+          )cpp",
+          [](HoverInfo &HI) {
+            HI.Name = "data";
+            HI.Type = "char";
+            HI.Kind = index::SymbolKind::Field;
+            HI.NamespaceScope = "ObjC::"; // FIXME: fix it
+            HI.Definition = "char data";
+          }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
   Symbol IndexSym = func("indexSymbol");
   IndexSym.Documentation = "comment from index";
   SymbolSlab::Builder Symbols;
   Symbols.insert(IndexSym);
   auto Index =
       MemIndex::build(std::move(Symbols).build(), RefSlab(), RelationSlab());
 
   for (const auto &Case : Cases) {
     SCOPED_TRACE(Case.Code);
 
     Annotations T(Case.Code);
     TestTU TU = TestTU::withCode(T.code());
     TU.ExtraArgs.push_back("-std=c++17");
+    TU.ExtraArgs.push_back("-xobjective-c++");
+
     TU.ExtraArgs.push_back("-Wno-gnu-designator");
     // Types might be different depending on the target triplet, we chose a
     // fixed one to make sure tests passes on different platform.
     TU.ExtraArgs.push_back("--target=x86_64-pc-linux-gnu");
     auto AST = TU.build();
 
     auto H = getHover(AST, T.point(), format::getLLVMStyle(), Index.get());
     ASSERT_TRUE(H);

clang/include/clang/AST/Decl.h

 //===- Decl.h - Classes for representing declarations -----------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file defines the Decl subclasses.
return hasCapturedVLAType() ? static_cast<const VariableArrayType *>(
                                 : nullptr;
   }
 
   /// Set the captured variable length array type for this field.
   void setCapturedVLAType(const VariableArrayType *VLAType);
 
   /// Returns the parent of this field declaration, which
   /// is the struct in which this field is defined.
+  ///
+  /// Returns null if this is not a normal class/struct field declaration, e.g.
+  /// ObjCAtDefsFieldDecl, ObjCIvarDecl.
   const RecordDecl *getParent() const {
-    return cast<RecordDecl>(getDeclContext());
+    return dyn_cast<RecordDecl>(getDeclContext());
   }
 
   RecordDecl *getParent() {
-    return cast<RecordDecl>(getDeclContext());
+    return dyn_cast<RecordDecl>(getDeclContext());
   }
 
   SourceRange getSourceRange() const override LLVM_READONLY;
 
   /// Retrieves the canonical declaration of this field.
   FieldDecl *getCanonicalDecl() override { return getFirstDecl(); }
   const FieldDecl *getCanonicalDecl() const { return getFirstDecl(); }