jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed.
Looking at the extant API's we do seem to prefer "const CompilerType &" over "const CompilerType" so it seems more consistent to choose that. Other than that, this looks fine to me. ================ Comment at: include/lldb/Symbol/ClangASTContext.h:909 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); ---------------- clayborg wrote: > teemperor wrote: > > Can we pass `enum_type` as const ref? > CompilerType instances are two pointers. Pretty cheap to copy. We're not entirely consistent, but a quick glance through headers show us almost always choosing to pass "const CompilerType &" over "const CompilerType". ================ Comment at: packages/Python/lldbsuite/test/expression_command/radar_43822994/TestScopedEnumType.py:42 + ## integral is not implicitly convertible to a scoped enum + value = frame.EvaluateExpression("1 == Foo::FooBar") + self.assertTrue(value.IsValid()) ---------------- davide wrote: > This clearly looks like can be an inline test (and probably would make the > thing more readable, IMHO). The form of test to use is up to the test writer, seems to me. This test is not at all hard to read. https://reviews.llvm.org/D54003 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits