shafik created this revision.
shafik added reviewers: aprantl, teemperor.
Currently the heuristic used in `ClangASTContext::CreateRecordType` to identify
an anonymous class is that there is that `name` is a `nullptr` or simply a null
terminator. This heuristic is not accurate since it will also sweep up unnamed
classes and lambdas. The improved heuristic relies on the requirement that an
anonymous class must be contained within a class.
The motivator for this fix is that we currently see crashes during code
completion inside lambdas and member functions of unnamed classes not within a
class. This heuristic fix removes these cases but leaves unnamed classes within
a class incorrectly labeled anonymous.
We will fix this problem long-term by implementing `DW_AT_export_symbols` which
will allow us to accurately identify anonymous classes without a heuristic.
https://reviews.llvm.org/D66175
Files:
packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/TestCompletionCrashInLambda.py
source/Symbol/ClangASTContext.cpp
Index: source/Symbol/ClangASTContext.cpp
===================================================================
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -1508,8 +1508,25 @@
*ast, (TagDecl::TagKind)kind, decl_ctx, SourceLocation(),
SourceLocation(), is_anonymous ? nullptr : &ast->Idents.get(name));
- if (is_anonymous)
- decl->setAnonymousStructOrUnion(true);
+ if (is_anonymous) {
+ // The current heuristic for checking if a CXXRecordDecl is anonymous is if
+ // there is no name or if it is just a null terminator.
+ // This is not accurate since currently this can sweep up both unnamed
+ // classes and lambdas as anonymous classes, which they are not.
+ //
+ // 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.
+ //
+ // This fix will unfortunately still mislabel unnamed classes within a
class
+ // but this improves the situation greatly since getting this wrong in the
+ // other cases can lead to an assert in clang CodeCompletion since
+ // SemaAccess assumes the DeclContext of an anonymous class is a
+ // CXXRecordDecl.
+ if (CXXRecordDecl *record = dyn_cast<CXXRecordDecl>(decl_ctx)) {
+ decl->setAnonymousStructOrUnion(true);
+ }
+ }
if (decl) {
if (metadata)
Index:
packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/TestCompletionCrashInLambda.py
===================================================================
---
packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/TestCompletionCrashInLambda.py
+++
packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/TestCompletionCrashInLambda.py
@@ -1,4 +1,4 @@
from lldbsuite.test import lldbinline
from lldbsuite.test import decorators
-lldbinline.MakeInlineTest(__file__, globals(),
[decorators.skipIf(bugnumber="rdar://53755023")])
+lldbinline.MakeInlineTest(__file__, globals(), [])
Index: source/Symbol/ClangASTContext.cpp
===================================================================
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -1508,8 +1508,25 @@
*ast, (TagDecl::TagKind)kind, decl_ctx, SourceLocation(),
SourceLocation(), is_anonymous ? nullptr : &ast->Idents.get(name));
- if (is_anonymous)
- decl->setAnonymousStructOrUnion(true);
+ if (is_anonymous) {
+ // The current heuristic for checking if a CXXRecordDecl is anonymous is if
+ // there is no name or if it is just a null terminator.
+ // This is not accurate since currently this can sweep up both unnamed
+ // classes and lambdas as anonymous classes, which they are not.
+ //
+ // 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.
+ //
+ // This fix will unfortunately still mislabel unnamed classes within a class
+ // but this improves the situation greatly since getting this wrong in the
+ // other cases can lead to an assert in clang CodeCompletion since
+ // SemaAccess assumes the DeclContext of an anonymous class is a
+ // CXXRecordDecl.
+ if (CXXRecordDecl *record = dyn_cast<CXXRecordDecl>(decl_ctx)) {
+ decl->setAnonymousStructOrUnion(true);
+ }
+ }
if (decl) {
if (metadata)
Index: packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/TestCompletionCrashInLambda.py
===================================================================
--- packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/TestCompletionCrashInLambda.py
+++ packages/Python/lldbsuite/test/expression_command/completion-crash-lambda/TestCompletionCrashInLambda.py
@@ -1,4 +1,4 @@
from lldbsuite.test import lldbinline
from lldbsuite.test import decorators
-lldbinline.MakeInlineTest(__file__, globals(), [decorators.skipIf(bugnumber="rdar://53755023")])
+lldbinline.MakeInlineTest(__file__, globals(), [])
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
