shafik created this revision.
shafik added reviewers: jingham, davide, teemperor.
Herald added a subscriber: JDevlieghere.
Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove
redundant parameter which can be calculated from other parameter.
Both calling sites of the sites were incorrectly calculating the qualtype as
the underlying type unconditionally irrespective of whether the enum was
unscoped or scoped
https://reviews.llvm.org/D54003
Files:
include/lldb/Symbol/ClangASTContext.h
packages/Python/lldbsuite/test/expression_command/radar_43822994/Makefile
packages/Python/lldbsuite/test/expression_command/radar_43822994/TestScopedEnumType.py
packages/Python/lldbsuite/test/expression_command/radar_43822994/main.cpp
source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
source/Symbol/ClangASTContext.cpp
Index: source/Symbol/ClangASTContext.cpp
===================================================================
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -8848,43 +8848,53 @@
}
clang::EnumConstantDecl *ClangASTContext::AddEnumerationValueToEnumerationType(
- lldb::opaque_compiler_type_t type,
- const CompilerType &enumerator_clang_type, const Declaration &decl,
- const char *name, int64_t enum_value, uint32_t enum_value_bit_size) {
- if (type && enumerator_clang_type.IsValid() && name && name[0]) {
- clang::QualType enum_qual_type(GetCanonicalQualType(type));
-
- bool is_signed = false;
- enumerator_clang_type.IsIntegerType(is_signed);
- const clang::Type *clang_type = enum_qual_type.getTypePtr();
- if (clang_type) {
- const clang::EnumType *enutype =
- llvm::dyn_cast<clang::EnumType>(clang_type);
-
- if (enutype) {
- llvm::APSInt enum_llvm_apsint(enum_value_bit_size, is_signed);
- enum_llvm_apsint = enum_value;
- clang::EnumConstantDecl *enumerator_decl =
- clang::EnumConstantDecl::Create(
- *getASTContext(), enutype->getDecl(), clang::SourceLocation(),
- name ? &getASTContext()->Idents.get(name)
- : nullptr, // Identifier
- ClangUtil::GetQualType(enumerator_clang_type),
- nullptr, enum_llvm_apsint);
-
- if (enumerator_decl) {
- enutype->getDecl()->addDecl(enumerator_decl);
+ const CompilerType enum_type, const Declaration &decl, const char *name,
+ int64_t enum_value, uint32_t enum_value_bit_size) {
+
+ if (!enum_type || ConstString(name).IsEmpty())
+ return nullptr;
+
+ lldb::opaque_compiler_type_t enum_opaque_compiler_type =
+ enum_type.GetOpaqueQualType();
+
+ if (!enum_opaque_compiler_type)
+ return nullptr;
+
+ CompilerType underlying_type =
+ GetEnumerationIntegerType(enum_type.GetOpaqueQualType());
+
+ clang::QualType enum_qual_type(
+ GetCanonicalQualType(enum_opaque_compiler_type));
+
+ bool is_signed = false;
+ underlying_type.IsIntegerType(is_signed);
+ const clang::Type *clang_type = enum_qual_type.getTypePtr();
+
+ if (!clang_type)
+ return nullptr;
+
+ const clang::EnumType *enutype = llvm::dyn_cast<clang::EnumType>(clang_type);
+
+ if (!enutype)
+ return nullptr;
+
+ llvm::APSInt enum_llvm_apsint(enum_value_bit_size, is_signed);
+ enum_llvm_apsint = enum_value;
+ clang::EnumConstantDecl *enumerator_decl = clang::EnumConstantDecl::Create(
+ *getASTContext(), enutype->getDecl(), clang::SourceLocation(),
+ name ? &getASTContext()->Idents.get(name) : nullptr, // Identifier
+ clang::QualType(enutype, 0), nullptr, enum_llvm_apsint);
+
+ if (!enumerator_decl)
+ return nullptr;
+
+ enutype->getDecl()->addDecl(enumerator_decl);
#ifdef LLDB_CONFIGURATION_DEBUG
- VerifyDecl(enumerator_decl);
+ VerifyDecl(enumerator_decl);
#endif
- return enumerator_decl;
- }
- }
- }
- }
- return nullptr;
+ return enumerator_decl;
}
CompilerType
Index: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===================================================================
--- source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -1086,8 +1086,7 @@
uint32_t byte_size = m_ast.getASTContext()->getTypeSize(
ClangUtil::GetQualType(underlying_type));
auto enum_constant_decl = m_ast.AddEnumerationValueToEnumerationType(
- enum_type.GetOpaqueQualType(), underlying_type, decl, name.c_str(),
- raw_value, byte_size * 8);
+ enum_type, decl, name.c_str(), raw_value, byte_size * 8);
if (!enum_constant_decl)
return false;
Index: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2501,9 +2501,7 @@
if (name && name[0] && got_value) {
m_ast.AddEnumerationValueToEnumerationType(
- clang_type.GetOpaqueQualType(),
- m_ast.GetEnumerationIntegerType(clang_type.GetOpaqueQualType()),
- decl, name, enum_value, enumerator_byte_size * 8);
+ clang_type, decl, name, enum_value, enumerator_byte_size * 8);
++enumerators_added;
}
}
Index: packages/Python/lldbsuite/test/expression_command/radar_43822994/main.cpp
===================================================================
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/radar_43822994/main.cpp
@@ -0,0 +1,16 @@
+enum class Foo {
+ FooBar = 42
+};
+
+enum Bar {
+ BarBar = 3,
+ BarBarBar = 42
+};
+
+int main(int argc, const char **argv) {
+ Foo f = Foo::FooBar;
+ Bar b = BarBar;
+ bool b1 = f == Foo::FooBar;
+ bool b2 = b == BarBar;
+ return 0; // Set break point at this line.
+}
Index: packages/Python/lldbsuite/test/expression_command/radar_43822994/TestScopedEnumType.py
===================================================================
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/radar_43822994/TestScopedEnumType.py
@@ -0,0 +1,44 @@
+from __future__ import print_function
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ExprXValuePrintingTestCase(TestBase):
+
+ mydir = TestBase.compute_mydir(__file__)
+
+ def test(self):
+ self.build()
+
+ self.main_source = "main.cpp"
+ self.main_source_spec = lldb.SBFileSpec(self.main_source)
+ (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+ '// Set break point at this line.', self.main_source_spec)
+ frame = thread.GetFrameAtIndex(0)
+
+ self.expect("expr f == Foo::FooBar",
+ substrs=['(bool) $0 = true'])
+
+ value = frame.EvaluateExpression("f == Foo::FooBar")
+ self.assertTrue(value.IsValid())
+ self.assertTrue(value.GetError().Success())
+ self.assertEqual(value.GetValueAsUnsigned(), 1)
+
+ value = frame.EvaluateExpression("b == BarBar")
+ self.assertTrue(value.IsValid())
+ self.assertTrue(value.GetError().Success())
+ self.assertEqual(value.GetValueAsUnsigned(), 1)
+
+ ## b is not a Foo
+ value = frame.EvaluateExpression("b == Foo::FooBar")
+ self.assertTrue(value.IsValid())
+ self.assertFalse(value.GetError().Success())
+
+ ## integral is not implicitly convertible to a scoped enum
+ value = frame.EvaluateExpression("1 == Foo::FooBar")
+ self.assertTrue(value.IsValid())
+ self.assertFalse(value.GetError().Success())
Index: packages/Python/lldbsuite/test/expression_command/radar_43822994/Makefile
===================================================================
--- /dev/null
+++ packages/Python/lldbsuite/test/expression_command/radar_43822994/Makefile
@@ -0,0 +1,6 @@
+LEVEL = ../../make
+
+CXX_SOURCES := main.cpp
+CXXFLAGS += -std=c++11
+
+include $(LEVEL)/Makefile.rules
Index: include/lldb/Symbol/ClangASTContext.h
===================================================================
--- include/lldb/Symbol/ClangASTContext.h
+++ include/lldb/Symbol/ClangASTContext.h
@@ -906,9 +906,8 @@
// Modifying Enumeration types
//----------------------------------------------------------------------
clang::EnumConstantDecl *AddEnumerationValueToEnumerationType(
- lldb::opaque_compiler_type_t type,
- const CompilerType &enumerator_qual_type, const Declaration &decl,
- const char *name, int64_t enum_value, uint32_t enum_value_bit_size);
+ const CompilerType enum_type, const Declaration &decl, const char *name,
+ int64_t enum_value, uint32_t enum_value_bit_size);
CompilerType GetEnumerationIntegerType(lldb::opaque_compiler_type_t type);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
