labath added inline comments.
================ Comment at: lldb/source/Core/Value.cpp:667 -const ValueList &ValueList::operator=(const ValueList &rhs) { +const ValueList &ValueList::operator=(const ValueList &rhs) { // NOLINT(modernize-use-equals-default) m_values = rhs.m_values; ---------------- shafik wrote: > labath wrote: > > shafik wrote: > > > I have to look into why we return a `const &` here. We do this in a few > > > other places too. > > I don't think there's a good reason for that. Most people aren't aware that > > built-in assignment operators return lvalues. And some of the people who > > are aware of that think that it's a bad idea, so they make sure their > > operators don't do it... > It produced build errors, so some of the users are relying on this. I didn't > want to plumb into this since it was orthogonal to the change. I was curious to see what kind of errors could be produced by that change -- I didn't get any, so I committed (c484857b2e77721a4235b0e2d53d335c09fc6af3) my version. :) ================ Comment at: lldb/source/Host/macosx/cfcpp/CFCMutableArray.cpp:18 CFCMutableArray::CFCMutableArray(const CFCMutableArray &rhs) - : CFCReleaser<CFMutableArrayRef>(rhs) // NOTE: this won't make a copy of the + : CFCReleaser<CFMutableArrayRef>(rhs) // NOLINT(modernize-use-equals-default) + // NOTE: this won't make a copy of the ---------------- shafik wrote: > labath wrote: > > Why suppress this? > I wanted to preserve the comment since someone thought it was important. In that case, maybe you can just put the comment next to the `=default` clause. Now, this is beginning to look like there is something very subtle going on -- subtle enough to confuse the clang-tidy check. Which, of course, isn't true... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121844/new/ https://reviews.llvm.org/D121844 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits