I don't think anyone is ok with that. I just think that a better solution is to document them. Why handle at runtime what is known about at compile time?
On Tue, Sep 20, 2016 at 2:24 PM Zachary Turner <ztur...@google.com> wrote: > Well, but StringRef for example is well documented. So it seems to me > like there's an example of a perfectly used assert. It's documented that > you can't use null, and if you do it asserts. Just like strlen. > > The issue I have with "you can't ever assert" is that it brings it into an > absolute when it really shouldn't be. We already agreed (I think) that > certain things that are well documented can assert. But when we talk in > absolutes, it tends to sway people that they should always do that thing, > even when it's not the most appropriate solution. And I think some of that > shows in the LLDB codebase where you've got hugely complicated logic that > is very hard to follow, reason about, or test, because no assumptions are > ever made about any of the inputs. Even when they are internal inputs that > are entirely controlled by us. > > On Tue, Sep 20, 2016 at 2:19 PM Greg Clayton <gclay...@apple.com> wrote: > > Again, strlen is a stupid example as it is well documented. All of llvm > and clang are not. > > On Sep 20, 2016, at 1:59 PM, Zachary Turner <ztur...@google.com> wrote: > > > > > > > > On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton <gclay...@apple.com> wrote: > > > > > On Sep 20, 2016, at 1:45 PM, Zachary Turner <ztur...@google.com> > wrote: > > > > > > I do agree that asserts are sometimes used improperly. But who's to > say that the bug was the assert, and not the surrounding code? For > example, consider this code: > > > > > > assert(p); > > > int x = *p; > > > > Should be written as: > > > > assert(p); > > if (!p) > > do_something_correct(); > > else > > int x = *p; > > > > > > > > Should this assert also not be here in library code? I mean it's > obvious that the program is about to crash if p is invalid. Asserts should > mean "you're about to invoke undefined behavior", and a crash is *better* > than undefined behavior. It surfaces the problem so that you can't let it > slip under the radar, and it also alerts you to the point that the UB is > invoked, rather than later. > > > > > > What about this assert? > > > > > > assert(ptr); > > > int x = strlen(ptr); > > > > > > Surely that assert is ok right? Do we need to check whether ptr is > valid EVERY SINGLE TIME we invoke strlen, or any other function for that > matter? The code would be a disastrous mess. > > > > Again, check before you call if this is in a shared library! What is so > hard about that? It is called software that doesn't crash. > > > > assert(ptr) > > int x = ptr ? strlen(ptr) : 0; > > > > I find it hard to believe that you are arguing that you cannot EVER know > ANYTHING about the state of your program. :-/ > > > > This is like arguing that you should run a full heap integrity check > every time you perform a memory write, just to be sure you aren't about to > crash. > > > > If you make a std::vector<>, do we need to verify that its internal > pointer is not null before we write to it? Probably not, right? Why > not? Because it has a specification of how it works, and it is documented > that you can construct one, you can use it. > > > > It's ok to document how functions work, and it is ok to assume that > functions work the way they claim to work. > >
_______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev