[lldb] Check for RecordDecl instead of CXXRecordDecl where possible

[teemperor]

Summary

clang::RecordDecl is Clang's type for describing structs when parsing C while clang::CXXRecordDecl
is a subclass of RecordDecl that represents structs/classes when parsing C++. Any code that isn't
exclusive to C++ should use RecordDecl when doing RTTI checks on declarations to be compatible
to both C and C++. For example, when iterating over fields of a record, RecordDecl is the right type
while for iterating over base classes, CXXRecordDecl is the right type.

However, as we currently only create CXXRecordDecl in LLDB at the moment (caused by all parsers
being set to C++ mode), a few of checks slipped into the code base where we incorrectly check for
CXXRecordDecl even when doing logic that also applies to C. This causes that if we ever allow C ASTs in
LLDB's type system, these check will start failing.

This patch just moves the` CXXRecordDecl` casts I found to be too strict to RecordDecl. The patch is
NFC at the moment as we only have C++ types as described above, but it makes the code base less hostile
for types from a Clang instance that parses C.

[shafik]

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

1336

I am wondering if this was a bug since anonymous classes is only an extension in C++ but it is a normative part of C.

So were we not correctly annotating C anonymous structs? If so we should have a test for that.

If we were correctly labeling C anonymous structs, I wonder were we got that right?

see D66175 for more context.

lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp

bool ClangASTImporter::CanImport(const CompilerType &type) {
   // CompleteAndFetchChildren should get the Decl out and check for the
 
   clang::QualType qual_type(
       ClangUtil::GetCanonicalQualType(ClangUtil::RemoveFastQualifiers(type)));
 
   const clang::Type::TypeClass type_class = qual_type->getTypeClass();
   switch (type_class) {
   case clang::Type::Record: {
-    const clang::CXXRecordDecl *cxx_record_decl =
-        qual_type->getAsCXXRecordDecl();
-    if (cxx_record_decl) {
-      if (GetDeclOrigin(cxx_record_decl).Valid())
+    const clang::RecordDecl *record_decl = qual_type->getAsRecordDecl();
+    if (record_decl) {
+      if (GetDeclOrigin(record_decl).Valid())
         return true;
     }
   } break;
 
   case clang::Type::Enum: {
     clang::EnumDecl *enum_decl =
         llvm::cast<clang::EnumType>(qual_type)->getDecl();
     if (enum_decl) {
bool ClangASTImporter::Import(const CompilerType &type) {
   // CompleteAndFetchChildren should get the Decl out and check for the
 
   clang::QualType qual_type(
       ClangUtil::GetCanonicalQualType(ClangUtil::RemoveFastQualifiers(type)));
 
   const clang::Type::TypeClass type_class = qual_type->getTypeClass();
   switch (type_class) {
   case clang::Type::Record: {
-    const clang::CXXRecordDecl *cxx_record_decl =
-        qual_type->getAsCXXRecordDecl();
-    if (cxx_record_decl) {
-      if (GetDeclOrigin(cxx_record_decl).Valid())
+    const clang::RecordDecl *record_decl = qual_type->getAsRecordDecl();
+    if (record_decl) {
+      if (GetDeclOrigin(record_decl).Valid())
         return CompleteAndFetchChildren(qual_type);
     }
   } break;
 
   case clang::Type::Enum: {
     clang::EnumDecl *enum_decl =
         llvm::cast<clang::EnumType>(qual_type)->getDecl();
     if (enum_decl) {

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

if (!has_name) {
     //    struct {
     //      int z;
     //    } C;
     // }
     //
     // Anonymous classes is a GNU/MSVC extension that clang supports. It
     // requires the anonymous class be embedded within a class. So the new
     // heuristic verifies this condition.
-    if (isa<CXXRecordDecl>(decl_ctx) && exports_symbols)
+    if (isa<RecordDecl>(decl_ctx) && exports_symbols)
       decl->setAnonymousStructOrUnion(true);
   }
 
   if (decl) {
     if (metadata)
       SetMetadata(decl, *metadata);
 
     if (access_type != eAccessNone)

 
 CompilerType TypeSystemClang::CreateStructForIdentifier(
     ConstString type_name,
     const std::initializer_list<std::pair<const char *, CompilerType>>
         &type_fields,
     bool packed) {
   CompilerType type;
   if (!type_name.IsEmpty() &&
-      (type = GetTypeForIdentifier<clang::CXXRecordDecl>(type_name))
-          .IsValid()) {
+      (type = GetTypeForIdentifier<clang::RecordDecl>(type_name)).IsValid()) {
     lldbassert(0 && "Trying to create a type for an existing name");
     return type;
   }
 
   type = CreateRecordType(nullptr, OptionalClangModuleID(), lldb::eAccessPublic,
                           type_name.GetCString(), clang::TTK_Struct,
                           lldb::eLanguageTypeC);
   StartTagDeclarationDefinition(type);
   for (const auto &field : type_fields)
     AddFieldToRecordType(type, field.first, field.second, lldb::eAccessPublic,
                          0);
   if (packed)
     SetIsPacked(type);
   CompleteTagDeclarationDefinition(type);
   return type;
 }
 
 CompilerType TypeSystemClang::GetOrCreateStructForIdentifier(
     ConstString type_name,
     const std::initializer_list<std::pair<const char *, CompilerType>>
         &type_fields,
     bool packed) {
   CompilerType type;
-  if ((type = GetTypeForIdentifier<clang::CXXRecordDecl>(type_name)).IsValid())
+  if ((type = GetTypeForIdentifier<clang::RecordDecl>(type_name)).IsValid())
     return type;
 
   return CreateStructForIdentifier(type_name, type_fields, packed);
 }
 
 #pragma mark Enumeration Types
 
 CompilerType TypeSystemClang::CreateEnumerationType(
case clang::Type::VariableArray: {
     const clang::ArrayType *array_type =
         llvm::dyn_cast<clang::ArrayType>(qual_type.getTypePtr());
 
     if (array_type)
       return GetCompleteQualType(ast, array_type->getElementType(),
                                  allow_completion);
   } break;
   case clang::Type::Record: {
-    clang::CXXRecordDecl *cxx_record_decl = qual_type->getAsCXXRecordDecl();
-    if (cxx_record_decl) {
-      if (cxx_record_decl->hasExternalLexicalStorage()) {
-        const bool is_complete = cxx_record_decl->isCompleteDefinition();
+    clang::RecordDecl *record_decl = qual_type->getAsRecordDecl();
+    if (record_decl) {
+      if (record_decl->hasExternalLexicalStorage()) {
+        const bool is_complete = record_decl->isCompleteDefinition();
         const bool fields_loaded =
-            cxx_record_decl->hasLoadedFieldsFromExternalStorage();
+            record_decl->hasLoadedFieldsFromExternalStorage();
         if (is_complete && fields_loaded)
           return true;
 
         if (!allow_completion)
           return false;
 
         // Call the field_begin() accessor to for it to use the external source
         // to load the fields...
         clang::ExternalASTSource *external_ast_source =
             ast->getExternalSource();
         if (external_ast_source) {
-          external_ast_source->CompleteType(cxx_record_decl);
-          if (cxx_record_decl->isCompleteDefinition()) {
-            cxx_record_decl->field_begin();
-            cxx_record_decl->setHasLoadedFieldsFromExternalStorage(true);
+          external_ast_source->CompleteType(record_decl);
+          if (record_decl->isCompleteDefinition()) {
+            record_decl->field_begin();
+            record_decl->setHasLoadedFieldsFromExternalStorage(true);
           }
         }
       }
     }
     const clang::TagType *tag_type =
         llvm::cast<clang::TagType>(qual_type.getTypePtr());
     return !tag_type->isIncompleteType();
   } break;
bool TypeSystemClang::SetHasExternalStorage(lldb::opaque_compiler_type_t type,
   if (!type)
     return false;
 
   clang::QualType qual_type(RemoveWrappingTypes(GetCanonicalQualType(type)));
 
   const clang::Type::TypeClass type_class = qual_type->getTypeClass();
   switch (type_class) {
   case clang::Type::Record: {
-    clang::CXXRecordDecl *cxx_record_decl = qual_type->getAsCXXRecordDecl();
-    if (cxx_record_decl) {
-      cxx_record_decl->setHasExternalLexicalStorage(has_extern);
-      cxx_record_decl->setHasExternalVisibleStorage(has_extern);
+    clang::RecordDecl *record_decl = qual_type->getAsRecordDecl();
+    if (record_decl) {
+      record_decl->setHasExternalLexicalStorage(has_extern);
+      record_decl->setHasExternalVisibleStorage(has_extern);
       return true;
     }
   } break;
 
   case clang::Type::Enum: {
     clang::EnumDecl *enum_decl =
         llvm::cast<clang::EnumType>(qual_type)->getDecl();
     if (enum_decl) {
if (auto *cxx_record_decl = llvm::dyn_cast<CXXRecordDecl>(tag_decl)) {
       //  operator is defined as deleted.
       if (cxx_record_decl->hasUserDeclaredMoveConstructor() ||
           cxx_record_decl->hasUserDeclaredMoveAssignment()) {
         if (cxx_record_decl->needsImplicitCopyConstructor())
           cxx_record_decl->setImplicitCopyConstructorIsDeleted();
         if (cxx_record_decl->needsImplicitCopyAssignment())
           cxx_record_decl->setImplicitCopyAssignmentIsDeleted();
       }
-
-      if (!cxx_record_decl->isCompleteDefinition())
-        cxx_record_decl->completeDefinition();
-      cxx_record_decl->setHasLoadedFieldsFromExternalStorage(true);
-      cxx_record_decl->setHasExternalLexicalStorage(false);
-      cxx_record_decl->setHasExternalVisibleStorage(false);
+    }
+    if (auto *record_decl = llvm::dyn_cast<RecordDecl>(tag_decl)) {
+      if (!record_decl->isCompleteDefinition())
+        record_decl->completeDefinition();
+      record_decl->setHasLoadedFieldsFromExternalStorage(true);
+      record_decl->setHasExternalLexicalStorage(false);
+      record_decl->setHasExternalVisibleStorage(false);
       return true;
     }
   }
 
   const clang::EnumType *enutype = qual_type->getAs<clang::EnumType>();
 
   if (!enutype)
     return false;