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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
