lgtm
On 1 February 2017 at 11:01, Zachary Turner <ztur...@google.com> wrote: > Correct, I'm not doing any of the "long term" stuff now, I was just thinking > out loud with that. > > On Wed, Feb 1, 2017 at 10:49 AM Jim Ingham <jing...@apple.com> wrote: >> >> I don't think any of this should be terribly controversial. The "Long >> term" part of the proposals might be, but that's not what you're talking >> about here, right? >> >> These changes will presumably require adjustments to the Xcode project. >> If you can do that yourself, then that's great. If you need one of us to >> fix the projects, putting them up for review before checking in will give >> one of us a chance to apply the patch and fix the projects and then we can >> gang the two submissions so we don't leave trunk broken. Other than that >> I'm fine with you just checking in the changes. >> >> Jim >> >> >> > On Jan 31, 2017, at 7:58 PM, Zachary Turner <ztur...@google.com> wrote: >> > >> > Are you interested in seeing the followup patches for review (moving >> > classes from Core -> Utility etc), or does it sound reasonable just based >> > on >> > my description? >> > >> > On Tue, Jan 31, 2017 at 6:05 PM Jim Ingham <jing...@apple.com> wrote: >> > BTW, not to exclude the positive because it doesn't need discussing: all >> > the rest of the changes you were proposing seem appropriate and good to me. >> > Thanks for taking the trouble to clean this up. >> > >> > Jim >> > >> > > On Jan 31, 2017, at 5:45 PM, Zachary Turner <ztur...@google.com> >> > > wrote: >> > > >> > > Yea, there may be value in both. If i ever tried to do this I'd >> > > probably take the approach of converting all the obvious cases first that >> > > should enforce checking and then see what's left. Then we could use >> > > llvm:Error and lldb::Status, or something >> > > On Tue, Jan 31, 2017 at 5:39 PM Jim Ingham <jing...@apple.com> wrote: >> > > Yeah, I have no idea how you'd make sure that the SBError returned to >> > > Python in a Python SBValue was checked before it went out of scope, and >> > > I'm >> > > not sure it's our business to be enforcing that... So we're going to >> > > have >> > > to do something special for those errors. We also use the pattern where >> > > we >> > > return a count and an error, but ofttimes you don't care why you got >> > > nothing, so you just check the count. The error is informational. >> > > Making >> > > that obnoxious would also not be a plus. I actually think there's some >> > > value to having "programming errors that must be checked" and >> > > informational >> > > errors. Maybe we need both. >> > > >> > > Jim >> > > >> > > > On Jan 31, 2017, at 5:32 PM, Zachary Turner <ztur...@google.com> >> > > > wrote: >> > > > >> > > > llvm::Error only enforces checking at the point that it goes out of >> > > > scope. So the example you mention should be supported, as long as you >> > > > propagate the value all the way up and don't destroy it. There's also >> > > > ways >> > > > to explicitly ignore an Error (similar to casting to void to make the >> > > > compiler not warn about unused variables), so as a last resort we >> > > > could put >> > > > code like that in the SB implementation if we had to >> > > > On Tue, Jan 31, 2017 at 5:23 PM Jim Ingham <jing...@apple.com> >> > > > wrote: >> > > > I think we discussed this before, but we need an informational error >> > > > object. IIRC, the llvm::Error has to be checked. But for instance if >> > > > you >> > > > ask a value object to evaluate itself, but you've moved to a section >> > > > of code >> > > > where the variable has no location, then evaluating that value will >> > > > result >> > > > in an error. But that isn't an error the value object code needs to do >> > > > anything about. And it might go all the way up through the SB & Python >> > > > API's before it's appropriate to check the error. >> > > > >> > > > IIRC, the llvm::Error is one of those "you have to check it or you >> > > > get smacked by the compiler" dealies. That's appropriate for some of >> > > > our >> > > > uses of Error, but not all. >> > > > >> > > > Jim >> > > > >> > > > >> > > > > On Jan 31, 2017, at 4:01 PM, Zachary Turner via Phabricator via >> > > > > lldb-commits <lldb-commits@lists.llvm.org> wrote: >> > > > > >> > > > > zturner added a comment. >> > > > > >> > > > >> Move Error from Core -> Utility >> > > > > >> > > > > Also, I almost forgot. >> > > > > >> > > > > Long term: Delete and use `llvm::Error` >> > > > > >> > > > > >> > > > > https://reviews.llvm.org/D29359 >> > > > > >> > > > > >> > > > > >> > > > > _______________________________________________ >> > > > > lldb-commits mailing list >> > > > > lldb-commits@lists.llvm.org >> > > > > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits >> > > > >> > > >> > >> > _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
