[Lldb-commits] [lldb] [lldb] Remove lldbassert in AppleObjCTypeEncodingParser (PR #93332)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/93332

AppleObjCTypeEncodingParser::BuildObjCObjectPointerType currently contains an lldbassert to detect situations where we have a forward declaration without a definition. According to the accompanying comment, its purpose is to catch "weird cases" during test suite runs.

However, because this is an lldbassert, we show a scary message to our users who think this is a problem and report the issue to us. Unfortunately those reports aren't very actionable without a way to know the name of the type.

This patch changes the lldbassert to a regular assert and emits a log message to the types log when this happens.

rdar://127439898

>From 55fb6056564f156f83b387d05f23368e06b64b19 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Fri, 24 May 2024 11:24:09 -0700
Subject: [PATCH] [lldb] Remove lldbassert in AppleObjCTypeEncodingParser

AppleObjCTypeEncodingParser::BuildObjCObjectPointerType currently
contains an lldbassert to detect situations where we have a forward
declaration without a definition. According to the accompanying comment,
its purpose is to catch "weird cases" during test suite runs.

However, because this is an lldbassert, we show a scary message to our
users who think this is a problem and report the issue to us.
Unfortunately those reports aren't very actionable without a way to know
the name of the type.

This patch changes the lldbassert to a regular assert and emits a log
message to the types log when this happens.

rdar://127439898
---
 .../AppleObjCTypeEncodingParser.cpp               | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
index ca582cb1d5a46..4871c59faefcc 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
@@ -13,6 +13,8 @@
 #include "lldb/Symbol/CompilerType.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/StringLexer.h"
 
 #include "clang/Basic/TargetInfo.h"
@@ -234,12 +236,15 @@ clang::QualType AppleObjCTypeEncodingParser::BuildObjCObjectPointerType(
 
     auto types = decl_vendor->FindTypes(ConstString(name), /*max_matches*/ 1);
 
-    // The user can forward-declare something that has no definition.  The runtime
-    // doesn't prohibit this at all. This is a rare and very weird case.  We keep
-    // this assert in debug builds so we catch other weird cases.
-    lldbassert(!types.empty());
-    if (types.empty())
+    if (types.empty()) {
+      // The user can forward-declare something that has no definition. The
+      // runtime doesn't prohibit this at all. This is a rare and very weird
+      // case. Assert assert in debug builds so we catch other weird cases.
+      assert(false && "forward declaration without definition");
+      LLDB_LOG(GetLog(LLDBLog::Types),
+               "forward declaration without definition: {0}", name)
       return ast_ctx.getObjCIdType();
+    }
 
     return ClangUtil::GetQualType(types.front().GetPointerType());
   } else {