zturner added inline comments.
================ 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); ---------------- shafik wrote: > jingham wrote: > > 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". > That was a good catch, thank you! Definitely passing by const value doesn't make sense, but I also think passing by value is what we should do going forward. It doesn't necessarily have to be in this patch, but I don't see any reason to pass by reference in general, and it just adds an extra level of pointer indirection for no real benefit IMO. https://reviews.llvm.org/D54003 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
