Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

Just want to add new things as I think of them.

On Mon, Sep 19, 2016 at 1:18 PM Zachary Turner  wrote:

>
>
>1.
>
>lldb-cli -- lldb interactive command line.
>
>
A great way to test this would be have a tool as mentioned, and you pass an
lldb command to the tool.  It parses it and spits out the parameters it
deduced.  For example, you could do this:

lldb-cli --command="break set -n main"

And it could print out:

Command: Set Breakpoint
Type: ByFunctionName
Location: main

And you can run this through FileCheck in a standard lit test.  This is a
great way to test the functionality of the command line without relying on
output scraping.
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

The =delete overload of StringRef is also a great idea.  It just helped me
catch all the places where we were initializing global option tables from
const char *s

On Wed, Sep 21, 2016 at 2:28 PM Zachary Turner  wrote:

> On Wed, Sep 21, 2016 at 2:20 PM Greg Clayton  wrote:
>
>
> > On Sep 21, 2016, at 1:55 PM, Zachary Turner  wrote:
> >
> > :-/  The same thing happens if you write Foo  = *nullptr;   It's a
> reference.
>
> I might be a good idea to add an overloaded constructor for nullptr and
> void * and delete them so that we can't implicitly create a StringRef from
> nullptr or NULL and cause as crash as it wouldn't compile in the first
> place.
>
> >
> > Did you try StringRef::withNullAsEmpty()?
>
> I am still catching code conversion issues with things like:
>
> if (name == nullptr)
>
>
> Yea you have to handle it on a case by case basis.  What I've been doing
> is to look around the surrounding code.  If it's obvious that it's not null
> (because it was just checked above, or because it's .c_str() of something,
> or it came from a string literal, or whatever, then i just call
> llvm::StringRef(name);  If I don't know, even if I can probably guess, I do
> one of two things.  If it seems like a rabbit hole you don't want to
> venture into, just do withNullAsEmpty.  If it's a trivial utility function
> that doesn't pass the string through many layers of nested calls, go change
> that function to return a StringRef, then come back.
>
> Another thing that's helped me a lot (although it will make the code
> slightly verbose in the short term) is that any function I'm changing the
> signature of from const char* to StringRef, copy the declaration and
> =delete the const char* version.  This way the compiler won't allow the
> implicit conversion, and you'll be forced to fix up every callsite.  This
> is annoying because 99% of the time you end up explicitly constructing
> StringRefs from literals, which will obviously work, but it's the only way
> to guarantee that the conversion won't introduce null pointer errors.
>
> Once the code is more or less done, we can go back and remove all the
> =delete functions we added so you don't have to write explicit conversions
> anymore.
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

On Wed, Sep 21, 2016 at 2:20 PM Greg Clayton  wrote:

>
> > On Sep 21, 2016, at 1:55 PM, Zachary Turner  wrote:
> >
> > :-/  The same thing happens if you write Foo  = *nullptr;   It's a
> reference.
>
> I might be a good idea to add an overloaded constructor for nullptr and
> void * and delete them so that we can't implicitly create a StringRef from
> nullptr or NULL and cause as crash as it wouldn't compile in the first
> place.
>
> >
> > Did you try StringRef::withNullAsEmpty()?
>
> I am still catching code conversion issues with things like:
>
> if (name == nullptr)
>

Yea you have to handle it on a case by case basis.  What I've been doing is
to look around the surrounding code.  If it's obvious that it's not null
(because it was just checked above, or because it's .c_str() of something,
or it came from a string literal, or whatever, then i just call
llvm::StringRef(name);  If I don't know, even if I can probably guess, I do
one of two things.  If it seems like a rabbit hole you don't want to
venture into, just do withNullAsEmpty.  If it's a trivial utility function
that doesn't pass the string through many layers of nested calls, go change
that function to return a StringRef, then come back.

Another thing that's helped me a lot (although it will make the code
slightly verbose in the short term) is that any function I'm changing the
signature of from const char* to StringRef, copy the declaration and
=delete the const char* version.  This way the compiler won't allow the
implicit conversion, and you'll be forced to fix up every callsite.  This
is annoying because 99% of the time you end up explicitly constructing
StringRefs from literals, which will obviously work, but it's the only way
to guarantee that the conversion won't introduce null pointer errors.

Once the code is more or less done, we can go back and remove all the
=delete functions we added so you don't have to write explicit conversions
anymore.
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] LLDB Evolution - Final Form

[Greg Clayton via lldb-dev]

> On Sep 21, 2016, at 1:55 PM, Zachary Turner  wrote:
> 
> :-/  The same thing happens if you write Foo  = *nullptr;   It's a 
> reference.

I might be a good idea to add an overloaded constructor for nullptr and void * 
and delete them so that we can't implicitly create a StringRef from nullptr or 
NULL and cause as crash as it wouldn't compile in the first place.

> 
> Did you try StringRef::withNullAsEmpty()?

I am still catching code conversion issues with things like:

if (name == nullptr)

For normal string ref construction we can cut over to using withNullAsEmpty.

> 
> BTW what code are you converting?  I'm working on some of the options code as 
> we speak.

Its the cut over to using the DWARF parser in LLVM. There are many functions 
that were missing and I have added them and other things we had in our DWARF 
parser. Any DWARF attribute that is a "const char *" will now be returned as a 
llvm::StringRef. That has wide implications in all of the existing DWARF 
parsing code that was using "const char *" for things like names and mangled 
names and also the ClangASTContext functions, like say to create a RecordDecl, 
it now takes an llvm::StringRef as an argument.

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] LLDB Evolution - Final Form

[Greg Clayton via lldb-dev]

I am laughing as I convert code over to using StringRef and I get crashes:

if (name == NULL)

StringRef is nice enough to implicitly construct a StringRef from NULL or 
nullptr so that it can crash for me...

> On Sep 21, 2016, at 11:09 AM, Zachary Turner via lldb-dev 
>  wrote:
> 
> Adding another thing to my list (thanks to Mehdi and Eric Christopher for the 
> idea).
> 
> Apply libfuzzer to LLDB.  Details sparse on what parse of LLDB and how, but I 
> think it would be easy to come up with candidates.
> 
> On Mon, Sep 19, 2016 at 1:18 PM Zachary Turner  wrote:
> Following up with Kate's post from a few weeks ago, I think the dust has 
> settled on the code reformat and it went over pretty smoothly for the most 
> part.  So I thought it might be worth throwing out some ideas for where we go 
> from here.  I have a large list of ideas (more ideas than time, sadly) that 
> I've been collecting over the past few weeks, so I figured I would throw them 
> out in the open for discussion.
> 
> I’ve grouped the areas for improvement into 3 high level categories.
> 
>   • De-inventing the wheel - We should use more code from LLVM, and 
> delete code in LLDB where LLVM provides a solution.  In cases where there is 
> an LLVM thing that is *similar* to what we need, we should extend the LLVM 
> thing to support what we need, and then use it.  Following are some areas 
> I've identified.  This list is by no means complete.  For each one, I've 
> given a personal assessment of how likely it is to cause some (temporary) 
> hiccups, how much it would help us in the long run, and how difficult it 
> would be to do.  Without further ado:
>   • Use llvm::Regex instead of lldb::Regex
>   • llvm::Regex doesn’t support enhanced mode.  Could we 
> add support for this to llvm::Regex?
>   • Risk: 6
>   • Impact: 3
>   • Difficulty / Effort: 3  (5 if we have to add enhanced 
> mode support)
>   • Use llvm streams instead of lldb::StreamString
>   • Supports output re-targeting (stderr, stdout, 
> std::string, etc), printf style formatting, and type-safe streaming operators.
>   • Interoperates nicely with many existing llvm utility 
> classes
>   • Risk: 4
>   • Impact: 5
>   • Difficulty / Effort: 7
>   • Use llvm::Error instead of lldb::Error
>   • llvm::Error is an error class that *requires* you to 
> check whether it succeeded or it will assert.  In a way, it's similar to a 
> C++ exception, except that it doesn't come with the performance hit 
> associated with exceptions.  It's extensible, and can be easily extended to 
> support the various ways LLDB needs to construct errors and error messages.
>   • Would need to first rename lldb::Error to LLDBError 
> so that te conversion from LLDBError to llvm::Error could be done 
> incrementally.
>   • Risk: 7
>   • Impact: 7
>   • Difficulty / Effort: 8
>   • StringRef instead of const char *, len everywhere
>   • Can do most common string operations in a way that is 
> guaranteed to be safe.
>   • Reduces string manipulation algorithm complexity by 
> an order of magnitude.
>   • Can potentially eliminate tens of thousands of string 
> copies across the codebase.
>   • Simplifies code.
>   • Risk: 3
>   • Impact: 8
>   • Difficulty / Effort: 7
>   • ArrayRef instead of const void *, len everywhere
>   • Same analysis as StringRef
>   • MutableArrayRef instead of void *, len everywhere
>   • Same analysis as StringRef
>   • Delete ConstString, use a modified StringPool that is 
> thread-safe.
>   • StringPool is a non thread-safe version of 
> ConstString.
>   • Strings are internally refcounted so they can be 
> cleaned up when they are no longer used.  ConstStrings are a large source of 
> memory in LLDB, so ref-counting and removing stale strings has the potential 
> to be a huge savings.
>   • Risk: 2
>   • Impact: 9
>   • Difficulty / Effort: 6
>   • thread_local instead of lldb::ThreadLocal
>   • This fixes a number of bugs on Windows that cannot be 
> fixed otherwise, as they require compiler support.
>   • Some other compilers may not support this yet?
>   • Risk: 2
>   • Impact: 3
>   • Difficulty: 3
>   • Use llvm::cl for the command line arguments to the primary 

Re: [lldb-dev] LLDB Evolution - Final Form

[Greg Clayton via lldb-dev]

I agree that for case #1, we must handle that by checking the pointer. Same 
thing for #2. For #3 we just need to fix the bug in clang. Our case in more of 
a different issue.

The cause of most crashes for us is in the clang::ASTContext class as we try to 
create types from DWARF. clang::ASTContext is used mainly by the compiler and 
this has a very rigid set of assumptions it can make about what inputs it gets. 
We create types like enums, CXXRecordDecl, Typedefs, arrays, etc by making 
types in the clang::ASTContext as we parse the DWARF. In the debugger, we are 
often creating classes from DWARF where information can be missing or not 
emitted. The -gline-tables-only feature emits DWARF, but takes the contents of 
all functions and removes all debug info children. This means you might have a 
function definition for say "bool Foo::operator <(const Foo&)" that has no 
parameters because the DWARF is truncated. This is one example of things that 
would make clang::ASTContext crash as it says you must have arguments for an 
"operator <" function which is understandable. We found that and fix the assert 
crash. But there are many many things in clang::ASTContext that mostly work, 
but as soon as we get fancier DWARF from a variety of people and test every 
variation, we end up crashing due to assertions. And the functions on the 
clang::ASTContext and many of the clang::Decl and clang::Type stuff don't have 
a ton of documentation saying "you must satisfy X or I will assert". Many of 
the assertions do make sense and are the kinds of things you want to see when 
debugging the creation of new types while you have a debug build. But in 
production, we would rather be able to try and survive just about anything.


> On Sep 21, 2016, at 8:25 AM, Zachary Turner  wrote:
> 
> BTW, one more comment about this.  If you've got a situation where LLDB is 
> using LLVM in a way that makes LLDB crash, there are 3 possibilities:
> 
> 1) LLVM / Clang is vending a pointer and we're supposed to be checking it for 
> null.
> 2) We used the LLVM / Clang API incorrectly causing it to return us a null 
> pointer.
> 3) There is a bug in the LLVM / Clang API.
> 
> Case #1 is the only case where we checking for null is the correct fix.  
> Blindly adding a null check and calling it a day is making the code *worse* 
> IMO.  If it's case 2 then we need to understand where in LLDB we are doing 
> the wrong thing and fix that.  If it's case 3 we need to dive into the LLVM / 
> Clang code and fix the real problem (presumably with help from someone with 
> expertise).  But "not crashing" is not the same as "fixing the bug".
> 
> FWIW, I expect a lot of these types of problems to fall somewhere between 
> categories 2 and 3.  LLDB is one of (if not the only) user of the clang 
> external ast source so that entire subsystem is not as battle hardened as the 
> rest of clang / llvm.
> 
> On Tue, Sep 20, 2016 at 2:31 PM Greg Clayton  wrote:
> We should avoid crashing if there is a reasonable work around when the input 
> is bad. StringRef with NULL is easy, just put NULL and zero length and don't 
> crash. Just because it is documented, doesn't mean it needs to stay that way, 
> but I am not going to fight that battle.
> 
> We should make every effort to not crash if we can. If it is way too 
> difficult, document the issue and make sure clients know that this is the way 
> things are. StringRef does this and we accept it. Doesn't mean it won't crash 
> us. I just hate seeing the crash logs where we have:
> 
> StringRef s(die.GetName());
> 
> It shouldn't crash IMHO, but we know it does and we now code around it. Yes, 
> we put in code like:
> 
> StringRef s;
> const char *cstr = die.GetName();
> if (cstr)
> s = cstr;
> 
> Is this nice code? I am glad it makes it simple for the LLVM side, but I 
> would rather write:
> 
> StringRef s(die.GetName());
> 
> Maybe I will subclass llvm::StringRef as lldb::StringRef and override the 
> constructor.
> 
> 
> > On Sep 20, 2016, at 2:24 PM, Zachary Turner  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 

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

BTW, one more comment about this.  If you've got a situation where LLDB is
using LLVM in a way that makes LLDB crash, there are 3 possibilities:

1) LLVM / Clang is vending a pointer and we're supposed to be checking it
for null.
2) We used the LLVM / Clang API incorrectly causing it to return us a null
pointer.
3) There is a bug in the LLVM / Clang API.

Case #1 is the only case where we checking for null is the correct fix.
Blindly adding a null check and calling it a day is making the code *worse*
IMO.  If it's case 2 then we need to understand where in LLDB we are doing
the wrong thing and fix that.  If it's case 3 we need to dive into the LLVM
/ Clang code and fix the real problem (presumably with help from someone
with expertise).  But "not crashing" is not the same as "fixing the bug".

FWIW, I expect a lot of these types of problems to fall somewhere between
categories 2 and 3.  LLDB is one of (if not the only) user of the clang
external ast source so that entire subsystem is not as battle hardened as
the rest of clang / llvm.

On Tue, Sep 20, 2016 at 2:31 PM Greg Clayton  wrote:

> We should avoid crashing if there is a reasonable work around when the
> input is bad. StringRef with NULL is easy, just put NULL and zero length
> and don't crash. Just because it is documented, doesn't mean it needs to
> stay that way, but I am not going to fight that battle.
>
> We should make every effort to not crash if we can. If it is way too
> difficult, document the issue and make sure clients know that this is the
> way things are. StringRef does this and we accept it. Doesn't mean it won't
> crash us. I just hate seeing the crash logs where we have:
>
> StringRef s(die.GetName());
>
> It shouldn't crash IMHO, but we know it does and we now code around it.
> Yes, we put in code like:
>
> StringRef s;
> const char *cstr = die.GetName();
> if (cstr)
> s = cstr;
>
> Is this nice code? I am glad it makes it simple for the LLVM side, but I
> would rather write:
>
> StringRef s(die.GetName());
>
> Maybe I will subclass llvm::StringRef as lldb::StringRef and override the
> constructor.
>
>
> > On Sep 20, 2016, at 2:24 PM, Zachary Turner  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  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 
> wrote:
> > >
> > >
> > >
> > > On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton 
> wrote:
> > >
> > > > On Sep 20, 2016, at 1:45 PM, Zachary Turner 
> 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 

Re: [lldb-dev] LLDB Evolution - Final Form

[Greg Clayton via lldb-dev]

> On Sep 20, 2016, at 2:41 PM, Mehdi Amini  wrote:
> 
>> 
>> On Sep 20, 2016, at 2:31 PM, Greg Clayton  wrote:
>> 
>> We should avoid crashing if there is a reasonable work around when the input 
>> is bad. StringRef with NULL is easy, just put NULL and zero length and don't 
>> crash. Just because it is documented, doesn't mean it needs to stay that 
>> way, but I am not going to fight that battle.
>> 
>> We should make every effort to not crash if we can. If it is way too 
>> difficult, document the issue and make sure clients know that this is the 
>> way things are. StringRef does this and we accept it. Doesn't mean it won't 
>> crash us. I just hate seeing the crash logs where we have:
>> 
>> StringRef s(die.GetName());
>> 
>> It shouldn't crash IMHO, but we know it does and we now code around it. Yes, 
>> we put in code like:
>> 
>> StringRef s;
>> const char *cstr = die.GetName();
>> if (cstr)
>>   s = cstr;
>> 
>> Is this nice code?
> 
> Well no, it is indeed terrible: changing die.GetName() to return StringRef 
> make the code a lot safer.
> 
> When StringRef is used everywhere, this problem disappear by itself. 
> StringRef(const char *) is a very unsafe API, that is not intended to be used 
> *inside* the library (or very sporadically). So if you build a StringRef out 
> of a const char *, you’re supposed to be in a place that deals with 
> “uncontrolled” input and you need to sanitize it. 
> 
> (Also StringRef(const char *) is “costly": it calls strlen)

That is fine, but someone is still making the initial StringRef with the "const 
char *" so strlen is still being called. Strings from DWARF come from a string 
table so we will always call strlen. We might reduce the number of times strlen 
is called though and that is good.


> — 
> Mehdi
> 
> 
> 
>> I am glad it makes it simple for the LLVM side, but I would rather write:
>> 
>> StringRef s(die.GetName());
>> 
>> Maybe I will subclass llvm::StringRef as lldb::StringRef and override the 
>> constructor.
>> 
>> 
>>> On Sep 20, 2016, at 2:24 PM, Zachary Turner  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  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  wrote:
 
 
 
 On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  wrote:
 
> On Sep 20, 2016, at 1:45 PM, Zachary Turner  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 

Re: [lldb-dev] LLDB Evolution - Final Form

[Mehdi Amini via lldb-dev]

> On Sep 20, 2016, at 2:46 PM, Zachary Turner  wrote:
> 
> Occasionally (and in my experience *very* occasionally), you need to treat "" 
> as different from null.

return an Optional?

— 
Mehdi



>   But the frequency with which that is really necessary is much lower than 
> people realize.  It just seems high because of the prevalence of raw pointers.
> 
> For every other case, you can use withNullAsEmpty()  (mostly used as a helper 
> for porting and when dealing with native APIs, which is to say quite a bit 
> right now, but in the long run, almost never (evidenced by the fact that it 
> only got added to LLVM a few days ago and nobody has ever needed it).
> 
> On Tue, Sep 20, 2016 at 2:41 PM Mehdi Amini  > wrote:
> 
> > On Sep 20, 2016, at 2:31 PM, Greg Clayton  > > wrote:
> >
> > We should avoid crashing if there is a reasonable work around when the 
> > input is bad. StringRef with NULL is easy, just put NULL and zero length 
> > and don't crash. Just because it is documented, doesn't mean it needs to 
> > stay that way, but I am not going to fight that battle.
> >
> > We should make every effort to not crash if we can. If it is way too 
> > difficult, document the issue and make sure clients know that this is the 
> > way things are. StringRef does this and we accept it. Doesn't mean it won't 
> > crash us. I just hate seeing the crash logs where we have:
> >
> > StringRef s(die.GetName());
> >
> > It shouldn't crash IMHO, but we know it does and we now code around it. 
> > Yes, we put in code like:
> >
> > StringRef s;
> > const char *cstr = die.GetName();
> > if (cstr)
> >s = cstr;
> >
> > Is this nice code?
> 
> Well no, it is indeed terrible: changing die.GetName() to return StringRef 
> make the code a lot safer.
> 
> When StringRef is used everywhere, this problem disappear by itself. 
> StringRef(const char *) is a very unsafe API, that is not intended to be used 
> *inside* the library (or very sporadically). So if you build a StringRef out 
> of a const char *, you’re supposed to be in a place that deals with 
> “uncontrolled” input and you need to sanitize it.
> 
> (Also StringRef(const char *) is “costly": it calls strlen)
> 
> —
> Mehdi
> 
> 
> 
> > I am glad it makes it simple for the LLVM side, but I would rather write:
> >
> > StringRef s(die.GetName());
> >
> > Maybe I will subclass llvm::StringRef as lldb::StringRef and override the 
> > constructor.
> >
> >
> >> On Sep 20, 2016, at 2:24 PM, Zachary Turner  >> > 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  >> > 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  >>> > wrote:
> >>>
> >>>
> >>>
> >>> On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  >>> > wrote:
> >>>
>  On Sep 20, 2016, at 1:45 PM, Zachary Turner   > 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.
> 

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

Occasionally (and in my experience *very* occasionally), you need to treat
"" as different from null.  But the frequency with which that is really
necessary is much lower than people realize.  It just seems high because of
the prevalence of raw pointers.

For every other case, you can use withNullAsEmpty()  (mostly used as a
helper for porting and when dealing with native APIs, which is to say quite
a bit right now, but in the long run, almost never (evidenced by the fact
that it only got added to LLVM a few days ago and nobody has ever needed
it).

On Tue, Sep 20, 2016 at 2:41 PM Mehdi Amini  wrote:

>
> > On Sep 20, 2016, at 2:31 PM, Greg Clayton  wrote:
> >
> > We should avoid crashing if there is a reasonable work around when the
> input is bad. StringRef with NULL is easy, just put NULL and zero length
> and don't crash. Just because it is documented, doesn't mean it needs to
> stay that way, but I am not going to fight that battle.
> >
> > We should make every effort to not crash if we can. If it is way too
> difficult, document the issue and make sure clients know that this is the
> way things are. StringRef does this and we accept it. Doesn't mean it won't
> crash us. I just hate seeing the crash logs where we have:
> >
> > StringRef s(die.GetName());
> >
> > It shouldn't crash IMHO, but we know it does and we now code around it.
> Yes, we put in code like:
> >
> > StringRef s;
> > const char *cstr = die.GetName();
> > if (cstr)
> >s = cstr;
> >
> > Is this nice code?
>
> Well no, it is indeed terrible: changing die.GetName() to return StringRef
> make the code a lot safer.
>
> When StringRef is used everywhere, this problem disappear by itself.
> StringRef(const char *) is a very unsafe API, that is not intended to be
> used *inside* the library (or very sporadically). So if you build a
> StringRef out of a const char *, you’re supposed to be in a place that
> deals with “uncontrolled” input and you need to sanitize it.
>
> (Also StringRef(const char *) is “costly": it calls strlen)
>
> —
> Mehdi
>
>
>
> > I am glad it makes it simple for the LLVM side, but I would rather write:
> >
> > StringRef s(die.GetName());
> >
> > Maybe I will subclass llvm::StringRef as lldb::StringRef and override
> the constructor.
> >
> >
> >> On Sep 20, 2016, at 2:24 PM, Zachary Turner  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 
> 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 
> wrote:
> >>>
> >>>
> >>>
> >>> On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton 
> wrote:
> >>>
>  On Sep 20, 2016, at 1:45 PM, Zachary Turner 
> 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) 

Re: [lldb-dev] LLDB Evolution - Final Form

[Mehdi Amini via lldb-dev]

> On Sep 20, 2016, at 2:31 PM, Greg Clayton  wrote:
> 
> We should avoid crashing if there is a reasonable work around when the input 
> is bad. StringRef with NULL is easy, just put NULL and zero length and don't 
> crash. Just because it is documented, doesn't mean it needs to stay that way, 
> but I am not going to fight that battle.
> 
> We should make every effort to not crash if we can. If it is way too 
> difficult, document the issue and make sure clients know that this is the way 
> things are. StringRef does this and we accept it. Doesn't mean it won't crash 
> us. I just hate seeing the crash logs where we have:
> 
> StringRef s(die.GetName());
> 
> It shouldn't crash IMHO, but we know it does and we now code around it. Yes, 
> we put in code like:
> 
> StringRef s;
> const char *cstr = die.GetName();
> if (cstr)
>s = cstr;
> 
> Is this nice code?

Well no, it is indeed terrible: changing die.GetName() to return StringRef make 
the code a lot safer.

When StringRef is used everywhere, this problem disappear by itself. 
StringRef(const char *) is a very unsafe API, that is not intended to be used 
*inside* the library (or very sporadically). So if you build a StringRef out of 
a const char *, you’re supposed to be in a place that deals with “uncontrolled” 
input and you need to sanitize it. 

(Also StringRef(const char *) is “costly": it calls strlen)

— 
Mehdi



> I am glad it makes it simple for the LLVM side, but I would rather write:
> 
> StringRef s(die.GetName());
> 
> Maybe I will subclass llvm::StringRef as lldb::StringRef and override the 
> constructor.
> 
> 
>> On Sep 20, 2016, at 2:24 PM, Zachary Turner  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  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  wrote:
>>> 
>>> 
>>> 
>>> On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  wrote:
>>> 
 On Sep 20, 2016, at 1:45 PM, Zachary Turner  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 

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

StringRef has `withNullAsEmpty` which I added a few days ago.  It will
return an empty StringRef.  seems to me that should solve most of those
kinds of problems.

On Tue, Sep 20, 2016 at 2:31 PM Greg Clayton  wrote:

> We should avoid crashing if there is a reasonable work around when the
> input is bad. StringRef with NULL is easy, just put NULL and zero length
> and don't crash. Just because it is documented, doesn't mean it needs to
> stay that way, but I am not going to fight that battle.
>
> We should make every effort to not crash if we can. If it is way too
> difficult, document the issue and make sure clients know that this is the
> way things are. StringRef does this and we accept it. Doesn't mean it won't
> crash us. I just hate seeing the crash logs where we have:
>
> StringRef s(die.GetName());
>
> It shouldn't crash IMHO, but we know it does and we now code around it.
> Yes, we put in code like:
>
> StringRef s;
> const char *cstr = die.GetName();
> if (cstr)
> s = cstr;
>
> Is this nice code? I am glad it makes it simple for the LLVM side, but I
> would rather write:
>
> StringRef s(die.GetName());
>
> Maybe I will subclass llvm::StringRef as lldb::StringRef and override the
> constructor.
>
>
> > On Sep 20, 2016, at 2:24 PM, Zachary Turner  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  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 
> wrote:
> > >
> > >
> > >
> > > On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton 
> wrote:
> > >
> > > > On Sep 20, 2016, at 1:45 PM, Zachary Turner 
> 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

Re: [lldb-dev] LLDB Evolution - Final Form

[Greg Clayton via lldb-dev]

We should avoid crashing if there is a reasonable work around when the input is 
bad. StringRef with NULL is easy, just put NULL and zero length and don't 
crash. Just because it is documented, doesn't mean it needs to stay that way, 
but I am not going to fight that battle.

We should make every effort to not crash if we can. If it is way too difficult, 
document the issue and make sure clients know that this is the way things are. 
StringRef does this and we accept it. Doesn't mean it won't crash us. I just 
hate seeing the crash logs where we have:

StringRef s(die.GetName());

It shouldn't crash IMHO, but we know it does and we now code around it. Yes, we 
put in code like:

StringRef s;
const char *cstr = die.GetName();
if (cstr)
s = cstr;

Is this nice code? I am glad it makes it simple for the LLVM side, but I would 
rather write:

StringRef s(die.GetName());

Maybe I will subclass llvm::StringRef as lldb::StringRef and override the 
constructor.


> On Sep 20, 2016, at 2:24 PM, Zachary Turner  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  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  wrote:
> >
> >
> >
> > On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  wrote:
> >
> > > On Sep 20, 2016, at 1:45 PM, Zachary Turner  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

Re: [lldb-dev] LLDB Evolution - Final Form

[Mehdi Amini via lldb-dev]

> On Sep 20, 2016, at 2:19 PM, Greg Clayton  wrote:
> 
> Again, strlen is a stupid example as it is well documented. All of llvm and 
> clang are not.

IMO that is:

1) A free claim that is easily defeated (to prove you wrong on *all* of LLVM 
being not document I just have to point you to one example where it is).
2) Not productive or constructive attitude. I’m not sure where you go with that…

—
Mehdi

>> On Sep 20, 2016, at 1:59 PM, Zachary Turner  wrote:
>> 
>> 
>> 
>> On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  wrote:
>> 
>>> On Sep 20, 2016, at 1:45 PM, Zachary Turner  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

Re: [lldb-dev] LLDB Evolution - Final Form

[Ed Maste via lldb-dev]

On 20 September 2016 at 17:25, Greg Clayton  wrote:
>
> Well the DWARF code was copied from LLDB into LLVM. The original person 
> didn't update LLDB to use it, so things diverged.

Yeah, sorry I didn't mention that explicitly. I do recall someone
pointed that out when this came up previously.

> I am actually in the process of fixing all of this as we speak, so don't do 
> any work on the DWARF parser. It will all be fixed in the next month or so!

Most excellent, thank you.
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

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  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  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  wrote:
> >
> >
> >
> > On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  wrote:
> >
> > > On Sep 20, 2016, at 1:45 PM, Zachary Turner 
> 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

Re: [lldb-dev] LLDB Evolution - Final Form

[Adrian McCarthy via lldb-dev]

My concern about this example:

void do_something(foo *p)
{
assert(p);
if (p)
p->crash();
}

Is that by skipping the operation when the pointer is null is that it's not
clear what it should do if it's precondition isn't met.  Sure, it won't
crash, but it's also not going to "do something."  This can lead to corrupt
state and postpones discovery of the bug.

If do_something is a public API, then, yes, you have to decide, document,
and implement what to do if the caller passes in a null pointer.  If it's
an internal API, then the silent elision of the crash merely hides the bug
possibly corrupts the debugger's state.  A corrupt debugging state seems
(to me) at least as bad as an obvious crash to the user.  Crashes are going
to get complained about and investigated.  Silently doing the wrong thing
just wastes everyone's time.

On Tue, Sep 20, 2016 at 1:59 PM, Zachary Turner via lldb-dev <
lldb-dev@lists.llvm.org> wrote:

>
>
> On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  wrote:
>
>>
>> > On Sep 20, 2016, at 1:45 PM, Zachary Turner  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
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  wrote:

>
> > On Sep 20, 2016, at 1:45 PM, Zachary Turner  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

Re: [lldb-dev] LLDB Evolution - Final Form

[Sean Callanan via lldb-dev]

My 2¢:

> assert(p);
> int x = *p;
> …

> assert(ptr);
> int x = strlen(ptr);

Both of these should either check for null, be in a situation where p is 
obviously good (e.g., p is data() from a stack-allocated std::vector), or use 
references.  The assertion to my mind is like an admission "I'm not 100% sure, 
so let me crash if I'm wrong..." – if we're making that admission and not doing 
error handling, we're already a little shady.

Sean
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] LLDB Evolution - Final Form

[Greg Clayton via lldb-dev]

> On Sep 20, 2016, at 1:45 PM, Zachary Turner  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;

> 
> If you think you've found an improper assert, the thing to do is raise it on 
> the proper mailing list and present your case of why you think it's an 
> improper assert.  asserting is not inherently wrong, but like anything, you 
> have to use it right.

The code to strlen() is well documented. All code in llvm and clang is not. 
Asserts fire off all the time for reasons the original author and others close 
to the code know about, but when the llvm and clang APIs are used by others, we 
don't know all of these cases. They aren't documented. The only way to find 
them is to crash when you run into them with questionable input. Not acceptable 
for a library.

> 
> On Tue, Sep 20, 2016 at 1:37 PM Ted Woodward  
> wrote:
> 
> From: lldb-dev [mailto:lldb-dev-boun...@lists.llvm.org] On Behalf Of Zachary 
> Turner via lldb-dev
> Sent: Tuesday, September 20, 2016 12:47 PM
> 
> > This kind of philisophical debate is probably worthy of a separate thread 
> > :)  That being said, I think asserts are typically used in places where the 
> > assert firing means "You're going to crash *anyway*"
> 
> This is emphatically NOT the case.
> 
> One of the first tasks I was given when I started at Qualcomm was to fix the 
> disassembler for Hexagon. It was a mess - it would assert if the disassembly 
> tables couldn't identify an opcode. Maybe that's fine for an assembler, 
> assert if you can't generate an opcode, but an unidentified opcode should 
> print a warning and move on. It's not a fatal error to disassemble data!
> 
> There are other instances of this in llvm. I agree with Greg - libraries 
> shouldn't assert.  Send an error back to the caller, and let the caller 
> handle it. A typo in my expression that lldb sends to clang shouldn't crash 
> my debug session.
> 
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
> Linux Foundation Collaborative Project
> 
> 

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

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 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.

If you think you've found an improper assert, the thing to do is raise it
on the proper mailing list and present your case of why you think it's an
improper assert.  asserting is not inherently wrong, but like anything, you
have to use it right.

On Tue, Sep 20, 2016 at 1:37 PM Ted Woodward 
wrote:

>
> From: lldb-dev [mailto:lldb-dev-boun...@lists.llvm.org] On Behalf Of
> Zachary Turner via lldb-dev
> Sent: Tuesday, September 20, 2016 12:47 PM
>
> > This kind of philisophical debate is probably worthy of a separate
> thread :)  That being said, I think asserts are typically used in places
> where the assert firing means "You're going to crash *anyway*"
>
> This is emphatically NOT the case.
>
> One of the first tasks I was given when I started at Qualcomm was to fix
> the disassembler for Hexagon. It was a mess - it would assert if the
> disassembly tables couldn't identify an opcode. Maybe that's fine for an
> assembler, assert if you can't generate an opcode, but an unidentified
> opcode should print a warning and move on. It's not a fatal error to
> disassemble data!
>
> There are other instances of this in llvm. I agree with Greg - libraries
> shouldn't assert.  Send an error back to the caller, and let the caller
> handle it. A typo in my expression that lldb sends to clang shouldn't crash
> my debug session.
>
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
> Linux Foundation Collaborative Project
>
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] LLDB Evolution - Final Form

[Greg Clayton via lldb-dev]

> On Sep 20, 2016, at 10:46 AM, Zachary Turner  wrote:
> 
> This kind of philisophical debate is probably worthy of a separate thread :)  
> That being said, I think asserts are typically used in places where the 
> assert firing means "You're going to crash *anyway*"
> 
> It's like trying to handle a bad alloc.  What are you going to do anyway?  
> You can crash now or you can crash later.  

With a StringRef being init'ed with NULL? You don't need to crash for that. In 
case where this is the only thing that you can do, that might be ok, but there 
are many many places where crashing isn't necessary.

> It's for guaranteeing the invariants of a function.  If you violate the 
> invariants of a function, then you might as well be passing null to strlen(), 
> or aliasing a value through a union, or dereferencing a null pointer, or 
> making a signed overflow, or accessing an array out of bounds.  It's all 
> equally undefined behavior, so crashing as soon as possible at least alerts 
> you to the source of the undefined behavior, rather than trying to hold on as 
> long as possible and then try to reconstruct the crash dynamics from the 
> crime scene after the fact, so to speak.
> 
> The example you gave about
> assert(p);
> if (p)
>   p->crash();
> 
> I disagree with.  There are two ways to look at this.  One way to look at it 
> is "I made my code safer by being defensive.  I don't know why p might be 
> null, but if it is, at least we won't crash".  Another way to look at it is 
> "I made my code more complex.  Now it's harder to reason about, harder to 
> analyze, and harder to test".
> 
> Which one is better?  We're in subjective territory, but peraonslly I lean 
> towards simple = better.  Simple code is easier to test because the are fewer 
> code paths, and I believe that if a code path isn't tested, then it's broken.

My only point is: if you are a library, you can't crash because you are unhappy 
with what you have. What that really means is take it out of process otherwise 
you will crash. I think the code should be as resilient as possible. I just 
moves the onus onto users of your library to figure out what makes you assert 
before you can possibly assert. And if there are rare cases that don't happen 
often, you find out about them when your users crash as they do something that 
causes the problem to be seen for the first time which doesn't make for a great 
experience. If all assertions were documented, then that would be one thing. 
But they aren't. You just run into them and must decipher what it means. 
> 
> On Tue, Sep 20, 2016 at 10:33 AM Greg Clayton  wrote:
> 
> > On Sep 19, 2016, at 2:46 PM, Mehdi Amini  wrote:
> >
> >>
> >> On Sep 19, 2016, at 2:34 PM, Greg Clayton  wrote:
> >>
> >>>
> >>> On Sep 19, 2016, at 2:20 PM, Mehdi Amini  wrote:
> >>>
> 
>  On Sep 19, 2016, at 2:02 PM, Greg Clayton via lldb-dev 
>   wrote:
> 
> 
> > On Sep 19, 2016, at 1:18 PM, Zachary Turner via lldb-dev 
> >  wrote:
> >
> > Following up with Kate's post from a few weeks ago, I think the dust 
> > has settled on the code reformat and it went over pretty smoothly for 
> > the most part.  So I thought it might be worth throwing out some ideas 
> > for where we go from here.  I have a large list of ideas (more ideas 
> > than time, sadly) that I've been collecting over the past few weeks, so 
> > I figured I would throw them out in the open for discussion.
> >
> > I’ve grouped the areas for improvement into 3 high level categories.
> >
> >   • De-inventing the wheel - We should use more code from LLVM, and 
> > delete code in LLDB where LLVM provides a solution.  In cases where 
> > there is an LLVM thing that is *similar* to what we need, we should 
> > extend the LLVM thing to support what we need, and then use it.  
> > Following are some areas I've identified.  This list is by no means 
> > complete.  For each one, I've given a personal assessment of how likely 
> > it is to cause some (temporary) hiccups, how much it would help us in 
> > the long run, and how difficult it would be to do.  Without further ado:
> >   • Use llvm::Regex instead of lldb::Regex
> >   • llvm::Regex doesn’t support enhanced mode.  Could 
> > we add support for this to llvm::Regex?
> >   • Risk: 6
> >   • Impact: 3
> >   • Difficulty / Effort: 3  (5 if we have to add 
> > enhanced mode support)
> 
>  As long as it supports all of the features, then this is fine. Otherwise 
>  we will need to modify the regular expressions to work with the LLVM 
>  version or back port any advances regex features we need into the LLVM 
>  regex parser.
> 
> > 

Re: [lldb-dev] LLDB Evolution - Final Form

[Mehdi Amini via lldb-dev]

> On Sep 20, 2016, at 10:33 AM, Greg Clayton  wrote:
> 
>> 
>> On Sep 19, 2016, at 2:46 PM, Mehdi Amini  wrote:
>> 
>>> 
>>> On Sep 19, 2016, at 2:34 PM, Greg Clayton  wrote:
>>> 
 
 On Sep 19, 2016, at 2:20 PM, Mehdi Amini  wrote:
 
> 
> On Sep 19, 2016, at 2:02 PM, Greg Clayton via lldb-dev 
>  wrote:
> 
> 
>> On Sep 19, 2016, at 1:18 PM, Zachary Turner via lldb-dev 
>>  wrote:
>> 
>> Following up with Kate's post from a few weeks ago, I think the dust has 
>> settled on the code reformat and it went over pretty smoothly for the 
>> most part.  So I thought it might be worth throwing out some ideas for 
>> where we go from here.  I have a large list of ideas (more ideas than 
>> time, sadly) that I've been collecting over the past few weeks, so I 
>> figured I would throw them out in the open for discussion.
>> 
>> I’ve grouped the areas for improvement into 3 high level categories.
>> 
>>  • De-inventing the wheel - We should use more code from LLVM, and 
>> delete code in LLDB where LLVM provides a solution.  In cases where 
>> there is an LLVM thing that is *similar* to what we need, we should 
>> extend the LLVM thing to support what we need, and then use it.  
>> Following are some areas I've identified.  This list is by no means 
>> complete.  For each one, I've given a personal assessment of how likely 
>> it is to cause some (temporary) hiccups, how much it would help us in 
>> the long run, and how difficult it would be to do.  Without further ado:
>>  • Use llvm::Regex instead of lldb::Regex
>>  • llvm::Regex doesn’t support enhanced mode.  Could we 
>> add support for this to llvm::Regex?
>>  • Risk: 6
>>  • Impact: 3
>>  • Difficulty / Effort: 3  (5 if we have to add enhanced 
>> mode support)
> 
> As long as it supports all of the features, then this is fine. Otherwise 
> we will need to modify the regular expressions to work with the LLVM 
> version or back port any advances regex features we need into the LLVM 
> regex parser. 
> 
>>  • Use llvm streams instead of lldb::StreamString
>>  • Supports output re-targeting (stderr, stdout, 
>> std::string, etc), printf style formatting, and type-safe streaming 
>> operators.
>>  • Interoperates nicely with many existing llvm utility 
>> classes
>>  • Risk: 4
>>  • Impact: 5
>>  • Difficulty / Effort: 7
> 
> I have worked extensively with both C++ streams and with printf. I don't 
> find the type safe streaming operators to be readable and a great way to 
> place things into streams. Part of my objection to this will be quelled 
> if the LLVM streams are not stateful like C++ streams are (add an extra 
> "std::hex" into the stream and suddenly other things down stream start 
> coming out in hex). As long as printf functionality is in the LLVM 
> streams I am ok with it.
> 
>>  • Use llvm::Error instead of lldb::Error
>>  • llvm::Error is an error class that *requires* you to 
>> check whether it succeeded or it will assert.  In a way, it's similar to 
>> a C++ exception, except that it doesn't come with the performance hit 
>> associated with exceptions.  It's extensible, and can be easily extended 
>> to support the various ways LLDB needs to construct errors and error 
>> messages.
>>  • Would need to first rename lldb::Error to LLDBError 
>> so that te conversion from LLDBError to llvm::Error could be done 
>> incrementally.
>>  • Risk: 7
>>  • Impact: 7
>>  • Difficulty / Effort: 8
> 
> We must be very careful here. Nothing can crash LLDB and adding something 
> that will be known to crash in some cases can only be changed if there is 
> a test that can guarantee that it won't crash. assert() calls in a shared 
> library like LLDB are not OK and shouldn't fire. Even if they do, when 
> asserts are off, then it shouldn't crash LLDB. We have made efforts to 
> only use asserts in LLDB for things that really can't happen, but for 
> things like constructing a StringRef with NULL is one example of why I 
> don't want to use certain classes in LLVM. If we can remove the crash 
> threat, then lets use them. But many people firmly believe that asserts 
> belong in llvm and clang code and I don't agree.
 
 I’m surprise by your aversion to assertions, what is your suggested 
 alternative? Are you expecting to check and handle every 

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

This kind of philisophical debate is probably worthy of a separate thread
:)  That being said, I think asserts are typically used in places where the
assert firing means "You're going to crash *anyway*"

It's like trying to handle a bad alloc.  What are you going to do anyway?
You can crash now or you can crash later.

It's for guaranteeing the invariants of a function.  If you violate the
invariants of a function, then you might as well be passing null to
strlen(), or aliasing a value through a union, or dereferencing a null
pointer, or making a signed overflow, or accessing an array out of bounds.
It's all equally undefined behavior, so crashing as soon as possible at
least alerts you to the source of the undefined behavior, rather than
trying to hold on as long as possible and then try to reconstruct the crash
dynamics from the crime scene after the fact, so to speak.

The example you gave about
assert(p);
if (p)
  p->crash();

I disagree with.  There are two ways to look at this.  One way to look at
it is "I made my code safer by being defensive.  I don't know why p might
be null, but if it is, at least we won't crash".  Another way to look at it
is "I made my code more complex.  Now it's harder to reason about, harder
to analyze, and harder to test".

Which one is better?  We're in subjective territory, but peraonslly I lean
towards simple = better.  Simple code is easier to test because the are
fewer code paths, and I believe that if a code path isn't tested, then it's
broken.

On Tue, Sep 20, 2016 at 10:33 AM Greg Clayton  wrote:

>
> > On Sep 19, 2016, at 2:46 PM, Mehdi Amini  wrote:
> >
> >>
> >> On Sep 19, 2016, at 2:34 PM, Greg Clayton  wrote:
> >>
> >>>
> >>> On Sep 19, 2016, at 2:20 PM, Mehdi Amini 
> wrote:
> >>>
> 
>  On Sep 19, 2016, at 2:02 PM, Greg Clayton via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> 
> 
> > On Sep 19, 2016, at 1:18 PM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >
> > Following up with Kate's post from a few weeks ago, I think the dust
> has settled on the code reformat and it went over pretty smoothly for the
> most part.  So I thought it might be worth throwing out some ideas for
> where we go from here.  I have a large list of ideas (more ideas than time,
> sadly) that I've been collecting over the past few weeks, so I figured I
> would throw them out in the open for discussion.
> >
> > I’ve grouped the areas for improvement into 3 high level categories.
> >
> >   • De-inventing the wheel - We should use more code from LLVM, and
> delete code in LLDB where LLVM provides a solution.  In cases where there
> is an LLVM thing that is *similar* to what we need, we should extend the
> LLVM thing to support what we need, and then use it.  Following are some
> areas I've identified.  This list is by no means complete.  For each one,
> I've given a personal assessment of how likely it is to cause some
> (temporary) hiccups, how much it would help us in the long run, and how
> difficult it would be to do.  Without further ado:
> >   • Use llvm::Regex instead of lldb::Regex
> >   • llvm::Regex doesn’t support enhanced mode.
> Could we add support for this to llvm::Regex?
> >   • Risk: 6
> >   • Impact: 3
> >   • Difficulty / Effort: 3  (5 if we have to add
> enhanced mode support)
> 
>  As long as it supports all of the features, then this is fine.
> Otherwise we will need to modify the regular expressions to work with the
> LLVM version or back port any advances regex features we need into the LLVM
> regex parser.
> 
> >   • Use llvm streams instead of lldb::StreamString
> >   • Supports output re-targeting (stderr, stdout,
> std::string, etc), printf style formatting, and type-safe streaming
> operators.
> >   • Interoperates nicely with many existing llvm
> utility classes
> >   • Risk: 4
> >   • Impact: 5
> >   • Difficulty / Effort: 7
> 
>  I have worked extensively with both C++ streams and with printf. I
> don't find the type safe streaming operators to be readable and a great way
> to place things into streams. Part of my objection to this will be quelled
> if the LLVM streams are not stateful like C++ streams are (add an extra
> "std::hex" into the stream and suddenly other things down stream start
> coming out in hex). As long as printf functionality is in the LLVM streams
> I am ok with it.
> 
> >   • Use llvm::Error instead of lldb::Error
> >   • llvm::Error is an error class that *requires*
> you to check whether it succeeded or it will assert.  In a way, it's
> similar to a C++ exception, except that it doesn't come with the
> performance hit associated 

Re: [lldb-dev] LLDB Evolution - Final Form

[Greg Clayton via lldb-dev]

> On Sep 19, 2016, at 2:46 PM, Mehdi Amini  wrote:
> 
>> 
>> On Sep 19, 2016, at 2:34 PM, Greg Clayton  wrote:
>> 
>>> 
>>> On Sep 19, 2016, at 2:20 PM, Mehdi Amini  wrote:
>>> 
 
 On Sep 19, 2016, at 2:02 PM, Greg Clayton via lldb-dev 
  wrote:
 
 
> On Sep 19, 2016, at 1:18 PM, Zachary Turner via lldb-dev 
>  wrote:
> 
> Following up with Kate's post from a few weeks ago, I think the dust has 
> settled on the code reformat and it went over pretty smoothly for the 
> most part.  So I thought it might be worth throwing out some ideas for 
> where we go from here.  I have a large list of ideas (more ideas than 
> time, sadly) that I've been collecting over the past few weeks, so I 
> figured I would throw them out in the open for discussion.
> 
> I’ve grouped the areas for improvement into 3 high level categories.
> 
>   • De-inventing the wheel - We should use more code from LLVM, and 
> delete code in LLDB where LLVM provides a solution.  In cases where there 
> is an LLVM thing that is *similar* to what we need, we should extend the 
> LLVM thing to support what we need, and then use it.  Following are some 
> areas I've identified.  This list is by no means complete.  For each one, 
> I've given a personal assessment of how likely it is to cause some 
> (temporary) hiccups, how much it would help us in the long run, and how 
> difficult it would be to do.  Without further ado:
>   • Use llvm::Regex instead of lldb::Regex
>   • llvm::Regex doesn’t support enhanced mode.  Could we 
> add support for this to llvm::Regex?
>   • Risk: 6
>   • Impact: 3
>   • Difficulty / Effort: 3  (5 if we have to add enhanced 
> mode support)
 
 As long as it supports all of the features, then this is fine. Otherwise 
 we will need to modify the regular expressions to work with the LLVM 
 version or back port any advances regex features we need into the LLVM 
 regex parser. 
 
>   • Use llvm streams instead of lldb::StreamString
>   • Supports output re-targeting (stderr, stdout, 
> std::string, etc), printf style formatting, and type-safe streaming 
> operators.
>   • Interoperates nicely with many existing llvm utility 
> classes
>   • Risk: 4
>   • Impact: 5
>   • Difficulty / Effort: 7
 
 I have worked extensively with both C++ streams and with printf. I don't 
 find the type safe streaming operators to be readable and a great way to 
 place things into streams. Part of my objection to this will be quelled if 
 the LLVM streams are not stateful like C++ streams are (add an extra 
 "std::hex" into the stream and suddenly other things down stream start 
 coming out in hex). As long as printf functionality is in the LLVM streams 
 I am ok with it.
 
>   • Use llvm::Error instead of lldb::Error
>   • llvm::Error is an error class that *requires* you to 
> check whether it succeeded or it will assert.  In a way, it's similar to 
> a C++ exception, except that it doesn't come with the performance hit 
> associated with exceptions.  It's extensible, and can be easily extended 
> to support the various ways LLDB needs to construct errors and error 
> messages.
>   • Would need to first rename lldb::Error to LLDBError 
> so that te conversion from LLDBError to llvm::Error could be done 
> incrementally.
>   • Risk: 7
>   • Impact: 7
>   • Difficulty / Effort: 8
 
 We must be very careful here. Nothing can crash LLDB and adding something 
 that will be known to crash in some cases can only be changed if there is 
 a test that can guarantee that it won't crash. assert() calls in a shared 
 library like LLDB are not OK and shouldn't fire. Even if they do, when 
 asserts are off, then it shouldn't crash LLDB. We have made efforts to 
 only use asserts in LLDB for things that really can't happen, but for 
 things like constructing a StringRef with NULL is one example of why I 
 don't want to use certain classes in LLVM. If we can remove the crash 
 threat, then lets use them. But many people firmly believe that asserts 
 belong in llvm and clang code and I don't agree.
>>> 
>>> I’m surprise by your aversion to assertions, what is your suggested 
>>> alternative? Are you expecting to check and handle every possible 
>>> invariants everywhere and recover (or signal an error) properly? That does 
>>> not seem practical, and it seems to me that it'd lead to just 

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

FWIW I added a function to StringRef the other day that looks like this:

static StringRef withNullAsEmpty(const char *Str) {
  return StringRef(Str ? Str : "");
}

I've been using this code in converting existing uses of const char * over
to StringRef.  It's not 100% what you want, but at least it's better than
nothing.

On Tue, Sep 20, 2016 at 10:26 AM Greg Clayton  wrote:

> But StringRef is a smart string wrapper and it is there for exactly this
> reason: to make string handling correct. So let us let it be smart and not
> crash if you make it with null and call .size() on it...
> > On Sep 19, 2016, at 2:37 PM, Zachary Turner  wrote:
> >
> > FWIW, strlen(nullptr) will also crash just as easily as
> StringRef(nullptr) will crash, so that one isn't a particularly convincing
> example of poorly used asserts, since the onus on the developer is exactly
> the same as with strlen.  That said, I still know where you're coming from
> :)
> >
> > On Mon, Sep 19, 2016 at 2:34 PM Greg Clayton  wrote:
> >
> > > On Sep 19, 2016, at 2:20 PM, Mehdi Amini 
> wrote:
> > >
> > >>
> > >> On Sep 19, 2016, at 2:02 PM, Greg Clayton via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> > >>
> > >>
> > >>> On Sep 19, 2016, at 1:18 PM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> > >>>
> > >>> Following up with Kate's post from a few weeks ago, I think the dust
> has settled on the code reformat and it went over pretty smoothly for the
> most part.  So I thought it might be worth throwing out some ideas for
> where we go from here.  I have a large list of ideas (more ideas than time,
> sadly) that I've been collecting over the past few weeks, so I figured I
> would throw them out in the open for discussion.
> > >>>
> > >>> I’ve grouped the areas for improvement into 3 high level categories.
> > >>>
> > >>> • De-inventing the wheel - We should use more code from LLVM,
> and delete code in LLDB where LLVM provides a solution.  In cases where
> there is an LLVM thing that is *similar* to what we need, we should extend
> the LLVM thing to support what we need, and then use it.  Following are
> some areas I've identified.  This list is by no means complete.  For each
> one, I've given a personal assessment of how likely it is to cause some
> (temporary) hiccups, how much it would help us in the long run, and how
> difficult it would be to do.  Without further ado:
> > >>> • Use llvm::Regex instead of lldb::Regex
> > >>> • llvm::Regex doesn’t support enhanced mode.
> Could we add support for this to llvm::Regex?
> > >>> • Risk: 6
> > >>> • Impact: 3
> > >>> • Difficulty / Effort: 3  (5 if we have to add
> enhanced mode support)
> > >>
> > >> As long as it supports all of the features, then this is fine.
> Otherwise we will need to modify the regular expressions to work with the
> LLVM version or back port any advances regex features we need into the LLVM
> regex parser.
> > >>
> > >>> • Use llvm streams instead of lldb::StreamString
> > >>> • Supports output re-targeting (stderr, stdout,
> std::string, etc), printf style formatting, and type-safe streaming
> operators.
> > >>> • Interoperates nicely with many existing llvm
> utility classes
> > >>> • Risk: 4
> > >>> • Impact: 5
> > >>> • Difficulty / Effort: 7
> > >>
> > >> I have worked extensively with both C++ streams and with printf. I
> don't find the type safe streaming operators to be readable and a great way
> to place things into streams. Part of my objection to this will be quelled
> if the LLVM streams are not stateful like C++ streams are (add an extra
> "std::hex" into the stream and suddenly other things down stream start
> coming out in hex). As long as printf functionality is in the LLVM streams
> I am ok with it.
> > >>
> > >>> • Use llvm::Error instead of lldb::Error
> > >>> • llvm::Error is an error class that *requires*
> you to check whether it succeeded or it will assert.  In a way, it's
> similar to a C++ exception, except that it doesn't come with the
> performance hit associated with exceptions.  It's extensible, and can be
> easily extended to support the various ways LLDB needs to construct errors
> and error messages.
> > >>> • Would need to first rename lldb::Error to
> LLDBError so that te conversion from LLDBError to llvm::Error could be done
> incrementally.
> > >>> • Risk: 7
> > >>> • Impact: 7
> > >>> • Difficulty / Effort: 8
> > >>
> > >> We must be very careful here. Nothing can crash LLDB and adding
> something that will be known to crash in some cases can only be changed if
> there is a test that can guarantee that it won't crash. 

Re: [lldb-dev] LLDB Evolution - Final Form

[Greg Clayton via lldb-dev]

But StringRef is a smart string wrapper and it is there for exactly this 
reason: to make string handling correct. So let us let it be smart and not 
crash if you make it with null and call .size() on it...
> On Sep 19, 2016, at 2:37 PM, Zachary Turner  wrote:
> 
> FWIW, strlen(nullptr) will also crash just as easily as StringRef(nullptr) 
> will crash, so that one isn't a particularly convincing example of poorly 
> used asserts, since the onus on the developer is exactly the same as with 
> strlen.  That said, I still know where you're coming from :)
> 
> On Mon, Sep 19, 2016 at 2:34 PM Greg Clayton  wrote:
> 
> > On Sep 19, 2016, at 2:20 PM, Mehdi Amini  wrote:
> >
> >>
> >> On Sep 19, 2016, at 2:02 PM, Greg Clayton via lldb-dev 
> >>  wrote:
> >>
> >>
> >>> On Sep 19, 2016, at 1:18 PM, Zachary Turner via lldb-dev 
> >>>  wrote:
> >>>
> >>> Following up with Kate's post from a few weeks ago, I think the dust has 
> >>> settled on the code reformat and it went over pretty smoothly for the 
> >>> most part.  So I thought it might be worth throwing out some ideas for 
> >>> where we go from here.  I have a large list of ideas (more ideas than 
> >>> time, sadly) that I've been collecting over the past few weeks, so I 
> >>> figured I would throw them out in the open for discussion.
> >>>
> >>> I’ve grouped the areas for improvement into 3 high level categories.
> >>>
> >>> • De-inventing the wheel - We should use more code from LLVM, and 
> >>> delete code in LLDB where LLVM provides a solution.  In cases where there 
> >>> is an LLVM thing that is *similar* to what we need, we should extend the 
> >>> LLVM thing to support what we need, and then use it.  Following are some 
> >>> areas I've identified.  This list is by no means complete.  For each one, 
> >>> I've given a personal assessment of how likely it is to cause some 
> >>> (temporary) hiccups, how much it would help us in the long run, and how 
> >>> difficult it would be to do.  Without further ado:
> >>> • Use llvm::Regex instead of lldb::Regex
> >>> • llvm::Regex doesn’t support enhanced mode.  Could 
> >>> we add support for this to llvm::Regex?
> >>> • Risk: 6
> >>> • Impact: 3
> >>> • Difficulty / Effort: 3  (5 if we have to add 
> >>> enhanced mode support)
> >>
> >> As long as it supports all of the features, then this is fine. Otherwise 
> >> we will need to modify the regular expressions to work with the LLVM 
> >> version or back port any advances regex features we need into the LLVM 
> >> regex parser.
> >>
> >>> • Use llvm streams instead of lldb::StreamString
> >>> • Supports output re-targeting (stderr, stdout, 
> >>> std::string, etc), printf style formatting, and type-safe streaming 
> >>> operators.
> >>> • Interoperates nicely with many existing llvm 
> >>> utility classes
> >>> • Risk: 4
> >>> • Impact: 5
> >>> • Difficulty / Effort: 7
> >>
> >> I have worked extensively with both C++ streams and with printf. I don't 
> >> find the type safe streaming operators to be readable and a great way to 
> >> place things into streams. Part of my objection to this will be quelled if 
> >> the LLVM streams are not stateful like C++ streams are (add an extra 
> >> "std::hex" into the stream and suddenly other things down stream start 
> >> coming out in hex). As long as printf functionality is in the LLVM streams 
> >> I am ok with it.
> >>
> >>> • Use llvm::Error instead of lldb::Error
> >>> • llvm::Error is an error class that *requires* you 
> >>> to check whether it succeeded or it will assert.  In a way, it's similar 
> >>> to a C++ exception, except that it doesn't come with the performance hit 
> >>> associated with exceptions.  It's extensible, and can be easily extended 
> >>> to support the various ways LLDB needs to construct errors and error 
> >>> messages.
> >>> • Would need to first rename lldb::Error to LLDBError 
> >>> so that te conversion from LLDBError to llvm::Error could be done 
> >>> incrementally.
> >>> • Risk: 7
> >>> • Impact: 7
> >>> • Difficulty / Effort: 8
> >>
> >> We must be very careful here. Nothing can crash LLDB and adding something 
> >> that will be known to crash in some cases can only be changed if there is 
> >> a test that can guarantee that it won't crash. assert() calls in a shared 
> >> library like LLDB are not OK and shouldn't fire. Even if they do, when 
> >> asserts are off, then it shouldn't crash LLDB. We have made efforts to 
> >> only use asserts in LLDB for things that really can't happen, but for 
> >> things like constructing a StringRef with NULL is one 

Re: [lldb-dev] LLDB Evolution - Final Form

[Jim Ingham via lldb-dev]

> On Sep 19, 2016, at 3:32 PM, Zachary Turner  wrote:
> 
> Ok, in that case I agree with you more.  We should test the scripting 
> interface.  It's part of the software, it should be treated as such.  100% on 
> board.  But if we find that it is lacking (or even just inconvenient) to test 
> the full capabilities of the debugger, we shouldn't force it to.

I would make an even stronger statement.  As lldb developers, if there's 
something we want to do and it isn't available, we go add it to lldb_private, 
and maybe wire it up to a command so we can use it.  That's the path of least 
resistance.  If it weren't for writing the testsuite using the SB API's, we 
wouldn't tend to write much code using it ourselves.  Writing test cases for 
the Python API in the testsuite is a nice forcing function to get US using the 
SB API's to solve actual problems.  So where reasonable, I think we actually 
SHOULD force ourselves to use it.

There are some things it is annoying to test from the outside, particularly 
small utility functions that you need to test error cases and the like that 
it's just hard to get the actual debugger to generate reliably.  Those are 
great candidates for white box type unit testing.  But the full capabilities of 
the debugger should either be available through the SB API's or they really 
aren't doing anybody any good.  So if the SB API's are not up to revealing the 
full capabilities of the debugger, we need to fix that, not fall back on using 
stuff only lldb developers can have access just so we can get our tests written 
more quickly.

Jim


> 
> All of our tests right now are full-scale integration tests.  Integration 
> tests are certainly helpful, but they aren't the entire world.  And they 
> shouldn't even be a first line of defense, but rather a 3rd or 4th line of 
> defense due to how general they are.  Like the example I gave earlier about 
> TestSigned and TestUnsigned on Windows, where one was failing and one was 
> passing, and the reason had nothing to do with signedness.  
> 
> On Mon, Sep 19, 2016 at 3:24 PM Jim Ingham  wrote:
> 
> > On Sep 19, 2016, at 3:19 PM, Zachary Turner  wrote:
> >
> > Obviously I defer to you on whether testing via the SB API is better than 
> > what GDB does or used to do.  But these are not the only two systems in the 
> > world.  Having used both LLDB and LLVM's test suite extensively, I still 
> > remain unconvinced that LLDB's testing situation could not be improved.  Do 
> > we improve it by doing what GDB did?  Obviously not.  Can we improve it 
> > further by doing something completely different than what GDB did *and* 
> > what LLDB currently does?  I remain convinced that we can.
> 
> I think you misread my message.  I said:
> 
> Having a Python interface so that developers can program the debugger is very 
> powerful.
> 
> As evidence, see all the work the gdb folks did to add one (you have to know 
> a little bit about how gdb works to see how hard this would be, but you can 
> imagine a debugger written in C primarily to dump text to a terminal, think 
> of how you would wrap that in Python and you'll get a sense.)
> 
> Now forget about gdb, that was only an example of how much some folks thought 
> a scripting interface was worth.
> 
> Instead, focus on this good thing, the scripting interface.  We should test 
> this thing that we think is valuable.
> 
> What's a good way to make sure we test that?
> 
> USE IT FOR TESTING.
> 
> Jim
> 
> 
> >
> >
> > On Mon, Sep 19, 2016 at 3:07 PM Jim Ingham  wrote:
> >
> > > On Sep 19, 2016, at 2:11 PM, Zachary Turner via lldb-dev 
> > >  wrote:
> > >
> > >
> > >
> > > On Mon, Sep 19, 2016 at 2:02 PM Greg Clayton  wrote:
> > >
> > > >   • Separate testing tools
> > > >   • One question that remains open is how to 
> > > > represent the complicated needs of a debugger in lit tests.  Part a) 
> > > > above covers the trivial cases, but what about the difficult cases?  In 
> > > > https://reviews.llvm.org/D24591 a number of ideas were discussed.  We 
> > > > started getting to this idea towards the end, about a separate tool 
> > > > which has an interface independent of the command line interface and 
> > > > which can be used to test.  lldb-mi was mentioned.  While I have 
> > > > serious concerns about lldb-mi due to its poorly written and tested 
> > > > codebase, I do agree in principle with the methodology.  In fact, this 
> > > > is the entire philosophy behind lit as used with LLVM, clang, lld, etc.
> > >
> > > We have a public API... Why are we going to go and spend _any_ time on 
> > > doing anything else? I just don't get it. What a waste of time. We have a 
> > > public API. Lets use it. Not simple enough for people? Tough. Testing a 
> > > debugger should be done using the public API except when we are actually 
> > > trying to test 

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

Ok, in that case I agree with you more.  We should test the scripting
interface.  It's part of the software, it should be treated as such.  100%
on board.  But if we find that it is lacking (or even just inconvenient) to
test the full capabilities of the debugger, we shouldn't force it to.

All of our tests right now are full-scale integration tests.  Integration
tests are certainly helpful, but they aren't the entire world.  And they
shouldn't even be a first line of defense, but rather a 3rd or 4th line of
defense due to how general they are.  Like the example I gave earlier about
TestSigned and TestUnsigned on Windows, where one was failing and one was
passing, and the reason had nothing to do with signedness.

On Mon, Sep 19, 2016 at 3:24 PM Jim Ingham  wrote:

>
> > On Sep 19, 2016, at 3:19 PM, Zachary Turner  wrote:
> >
> > Obviously I defer to you on whether testing via the SB API is better
> than what GDB does or used to do.  But these are not the only two systems
> in the world.  Having used both LLDB and LLVM's test suite extensively, I
> still remain unconvinced that LLDB's testing situation could not be
> improved.  Do we improve it by doing what GDB did?  Obviously not.  Can we
> improve it further by doing something completely different than what GDB
> did *and* what LLDB currently does?  I remain convinced that we can.
>
> I think you misread my message.  I said:
>
> Having a Python interface so that developers can program the debugger is
> very powerful.
>
> As evidence, see all the work the gdb folks did to add one (you have to
> know a little bit about how gdb works to see how hard this would be, but
> you can imagine a debugger written in C primarily to dump text to a
> terminal, think of how you would wrap that in Python and you'll get a
> sense.)
>
> Now forget about gdb, that was only an example of how much some folks
> thought a scripting interface was worth.
>
> Instead, focus on this good thing, the scripting interface.  We should
> test this thing that we think is valuable.
>
> What's a good way to make sure we test that?
>
> USE IT FOR TESTING.
>
> Jim
>
>
> >
> >
> > On Mon, Sep 19, 2016 at 3:07 PM Jim Ingham  wrote:
> >
> > > On Sep 19, 2016, at 2:11 PM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> > >
> > >
> > >
> > > On Mon, Sep 19, 2016 at 2:02 PM Greg Clayton 
> wrote:
> > >
> > > >   • Separate testing tools
> > > >   • One question that remains open is how to
> represent the complicated needs of a debugger in lit tests.  Part a) above
> covers the trivial cases, but what about the difficult cases?  In
> https://reviews.llvm.org/D24591 a number of ideas were discussed.  We
> started getting to this idea towards the end, about a separate tool which
> has an interface independent of the command line interface and which can be
> used to test.  lldb-mi was mentioned.  While I have serious concerns about
> lldb-mi due to its poorly written and tested codebase, I do agree in
> principle with the methodology.  In fact, this is the entire philosophy
> behind lit as used with LLVM, clang, lld, etc.
> > >
> > > We have a public API... Why are we going to go and spend _any_ time on
> doing anything else? I just don't get it. What a waste of time. We have a
> public API. Lets use it. Not simple enough for people? Tough. Testing a
> debugger should be done using the public API except when we are actually
> trying to test the LLDB command line in pexpect like tests. Those and only
> those are fine to covert over to using LIT and use text scraping. But 95%
> of our testing should be done using the API that our IDEs use. Using MI?
> Please no.
> > >
> > > FWIW, I agree with you about mi.
> > >
> > > That said, the public api is problematic.  Here you've got an API
> which, once you do something to, is set in stone forever.  And yet in order
> to test something, you have to add it to the API.  So now, in order to test
> something, you have to make a permanent change to a public API that can
> never be undone.
> > >
> > > It's also a public facing API, when a lot of the stuff  we need to be
> able to look at and inspect is not really suitable for public consumption.
> > >
> > > If something is a public API that can never be changed once it's
> added, then there should be an extremely strict review process in order to
> add something to it.  It should be VERY HARD to modify (it's currently not,
> which is a debate for another day, but anyway...).  And yet that's
> fundamentally incompatible with having tests be easy to write.  When it's
> hard to add the thing you need to add in order to write the test, then it's
> hard to write the test.
> > >
> > > Furthermore, with a system such as the one I proposed, you could even
> run tests with LLDB_DISABLE_PYTHON.  IDK about you, but that would be
> *amazing* from my point of view.  Even though I spent months making 

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

Obviously I defer to you on whether testing via the SB API is better than
what GDB does or used to do.  But these are not the only two systems in the
world.  Having used both LLDB and LLVM's test suite extensively, I still
remain unconvinced that LLDB's testing situation could not be improved.  Do
we improve it by doing what GDB did?  Obviously not.  Can we improve it
further by doing something completely different than what GDB did *and*
what LLDB currently does?  I remain convinced that we can.


On Mon, Sep 19, 2016 at 3:07 PM Jim Ingham  wrote:

>
> > On Sep 19, 2016, at 2:11 PM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >
> >
> >
> > On Mon, Sep 19, 2016 at 2:02 PM Greg Clayton  wrote:
> >
> > >   • Separate testing tools
> > >   • One question that remains open is how to
> represent the complicated needs of a debugger in lit tests.  Part a) above
> covers the trivial cases, but what about the difficult cases?  In
> https://reviews.llvm.org/D24591 a number of ideas were discussed.  We
> started getting to this idea towards the end, about a separate tool which
> has an interface independent of the command line interface and which can be
> used to test.  lldb-mi was mentioned.  While I have serious concerns about
> lldb-mi due to its poorly written and tested codebase, I do agree in
> principle with the methodology.  In fact, this is the entire philosophy
> behind lit as used with LLVM, clang, lld, etc.
> >
> > We have a public API... Why are we going to go and spend _any_ time on
> doing anything else? I just don't get it. What a waste of time. We have a
> public API. Lets use it. Not simple enough for people? Tough. Testing a
> debugger should be done using the public API except when we are actually
> trying to test the LLDB command line in pexpect like tests. Those and only
> those are fine to covert over to using LIT and use text scraping. But 95%
> of our testing should be done using the API that our IDEs use. Using MI?
> Please no.
> >
> > FWIW, I agree with you about mi.
> >
> > That said, the public api is problematic.  Here you've got an API which,
> once you do something to, is set in stone forever.  And yet in order to
> test something, you have to add it to the API.  So now, in order to test
> something, you have to make a permanent change to a public API that can
> never be undone.
> >
> > It's also a public facing API, when a lot of the stuff  we need to be
> able to look at and inspect is not really suitable for public consumption.
> >
> > If something is a public API that can never be changed once it's added,
> then there should be an extremely strict review process in order to add
> something to it.  It should be VERY HARD to modify (it's currently not,
> which is a debate for another day, but anyway...).  And yet that's
> fundamentally incompatible with having tests be easy to write.  When it's
> hard to add the thing you need to add in order to write the test, then it's
> hard to write the test.
> >
> > Furthermore, with a system such as the one I proposed, you could even
> run tests with LLDB_DISABLE_PYTHON.  IDK about you, but that would be
> *amazing* from my point of view.  Even though I spent months making Python
> 3 work, I would be happy to burn it all with fire and go back to just
> Python 2 if we had a viable testing strategy.
>
> It would not be amazing.  One of the primary benefits of lldb IS the
> Python API.  If you don't think that's wonderful in a debugger, go look at
> what effort it took to make that happen in gdb once lldb started providing
> it and you'll see that it was really highly motivated.  So switching our
> testing away from one of our very strong points would be a very bad choice
> IMHO.
>
> On the "should I add it to the SB API to test it or not question."  Most
> of the time, when I find I need to add an API in order write a test for
> something, the thing I'm trying to get my hands on is something that other
> people might be interested in as well.  If we go to the mode of adding
> affordances only for ourselves when writing for testing, we remove one of
> the times you should be asking yourself "should I add this to the SB API so
> other people can use it."  I'm fine with having an lldb-testing module with
> API's based on the same methodology as the SB API's.  If you think
> something you are adding is too esoteric to be part of the SB API put it
> there.  But by default put it into the SB API's.
>
> Jim
>
>
> > ___
> > lldb-dev mailing list
> > lldb-dev@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] LLDB Evolution - Final Form

[Jim Ingham via lldb-dev]

> On Sep 19, 2016, at 3:19 PM, Zachary Turner  wrote:
> 
> Obviously I defer to you on whether testing via the SB API is better than 
> what GDB does or used to do.  But these are not the only two systems in the 
> world.  Having used both LLDB and LLVM's test suite extensively, I still 
> remain unconvinced that LLDB's testing situation could not be improved.  Do 
> we improve it by doing what GDB did?  Obviously not.  Can we improve it 
> further by doing something completely different than what GDB did *and* what 
> LLDB currently does?  I remain convinced that we can.

I think you misread my message.  I said:

Having a Python interface so that developers can program the debugger is very 
powerful.

As evidence, see all the work the gdb folks did to add one (you have to know a 
little bit about how gdb works to see how hard this would be, but you can 
imagine a debugger written in C primarily to dump text to a terminal, think of 
how you would wrap that in Python and you'll get a sense.)

Now forget about gdb, that was only an example of how much some folks thought a 
scripting interface was worth.

Instead, focus on this good thing, the scripting interface.  We should test 
this thing that we think is valuable.

What's a good way to make sure we test that?

USE IT FOR TESTING.

Jim


> 
> 
> On Mon, Sep 19, 2016 at 3:07 PM Jim Ingham  wrote:
> 
> > On Sep 19, 2016, at 2:11 PM, Zachary Turner via lldb-dev 
> >  wrote:
> >
> >
> >
> > On Mon, Sep 19, 2016 at 2:02 PM Greg Clayton  wrote:
> >
> > >   • Separate testing tools
> > >   • One question that remains open is how to 
> > > represent the complicated needs of a debugger in lit tests.  Part a) 
> > > above covers the trivial cases, but what about the difficult cases?  In 
> > > https://reviews.llvm.org/D24591 a number of ideas were discussed.  We 
> > > started getting to this idea towards the end, about a separate tool which 
> > > has an interface independent of the command line interface and which can 
> > > be used to test.  lldb-mi was mentioned.  While I have serious concerns 
> > > about lldb-mi due to its poorly written and tested codebase, I do agree 
> > > in principle with the methodology.  In fact, this is the entire 
> > > philosophy behind lit as used with LLVM, clang, lld, etc.
> >
> > We have a public API... Why are we going to go and spend _any_ time on 
> > doing anything else? I just don't get it. What a waste of time. We have a 
> > public API. Lets use it. Not simple enough for people? Tough. Testing a 
> > debugger should be done using the public API except when we are actually 
> > trying to test the LLDB command line in pexpect like tests. Those and only 
> > those are fine to covert over to using LIT and use text scraping. But 95% 
> > of our testing should be done using the API that our IDEs use. Using MI? 
> > Please no.
> >
> > FWIW, I agree with you about mi.
> >
> > That said, the public api is problematic.  Here you've got an API which, 
> > once you do something to, is set in stone forever.  And yet in order to 
> > test something, you have to add it to the API.  So now, in order to test 
> > something, you have to make a permanent change to a public API that can 
> > never be undone.
> >
> > It's also a public facing API, when a lot of the stuff  we need to be able 
> > to look at and inspect is not really suitable for public consumption.
> >
> > If something is a public API that can never be changed once it's added, 
> > then there should be an extremely strict review process in order to add 
> > something to it.  It should be VERY HARD to modify (it's currently not, 
> > which is a debate for another day, but anyway...).  And yet that's 
> > fundamentally incompatible with having tests be easy to write.  When it's 
> > hard to add the thing you need to add in order to write the test, then it's 
> > hard to write the test.
> >
> > Furthermore, with a system such as the one I proposed, you could even run 
> > tests with LLDB_DISABLE_PYTHON.  IDK about you, but that would be *amazing* 
> > from my point of view.  Even though I spent months making Python 3 work, I 
> > would be happy to burn it all with fire and go back to just Python 2 if we 
> > had a viable testing strategy.
> 
> It would not be amazing.  One of the primary benefits of lldb IS the Python 
> API.  If you don't think that's wonderful in a debugger, go look at what 
> effort it took to make that happen in gdb once lldb started providing it and 
> you'll see that it was really highly motivated.  So switching our testing 
> away from one of our very strong points would be a very bad choice IMHO.
> 
> On the "should I add it to the SB API to test it or not question."  Most of 
> the time, when I find I need to add an API in order write a test for 
> something, the thing I'm trying to get my hands on is something that other 
> people might 

Re: [lldb-dev] LLDB Evolution - Final Form

[Mehdi Amini via lldb-dev]

> On Sep 19, 2016, at 2:34 PM, Greg Clayton  wrote:
> 
>> 
>> On Sep 19, 2016, at 2:20 PM, Mehdi Amini  wrote:
>> 
>>> 
>>> On Sep 19, 2016, at 2:02 PM, Greg Clayton via lldb-dev 
>>>  wrote:
>>> 
>>> 
 On Sep 19, 2016, at 1:18 PM, Zachary Turner via lldb-dev 
  wrote:
 
 Following up with Kate's post from a few weeks ago, I think the dust has 
 settled on the code reformat and it went over pretty smoothly for the most 
 part.  So I thought it might be worth throwing out some ideas for where we 
 go from here.  I have a large list of ideas (more ideas than time, sadly) 
 that I've been collecting over the past few weeks, so I figured I would 
 throw them out in the open for discussion.
 
 I’ve grouped the areas for improvement into 3 high level categories.
 
• De-inventing the wheel - We should use more code from LLVM, and 
 delete code in LLDB where LLVM provides a solution.  In cases where there 
 is an LLVM thing that is *similar* to what we need, we should extend the 
 LLVM thing to support what we need, and then use it.  Following are some 
 areas I've identified.  This list is by no means complete.  For each one, 
 I've given a personal assessment of how likely it is to cause some 
 (temporary) hiccups, how much it would help us in the long run, and how 
 difficult it would be to do.  Without further ado:
• Use llvm::Regex instead of lldb::Regex
• llvm::Regex doesn’t support enhanced mode.  Could we 
 add support for this to llvm::Regex?
• Risk: 6
• Impact: 3
• Difficulty / Effort: 3  (5 if we have to add enhanced 
 mode support)
>>> 
>>> As long as it supports all of the features, then this is fine. Otherwise we 
>>> will need to modify the regular expressions to work with the LLVM version 
>>> or back port any advances regex features we need into the LLVM regex 
>>> parser. 
>>> 
• Use llvm streams instead of lldb::StreamString
• Supports output re-targeting (stderr, stdout, 
 std::string, etc), printf style formatting, and type-safe streaming 
 operators.
• Interoperates nicely with many existing llvm utility 
 classes
• Risk: 4
• Impact: 5
• Difficulty / Effort: 7
>>> 
>>> I have worked extensively with both C++ streams and with printf. I don't 
>>> find the type safe streaming operators to be readable and a great way to 
>>> place things into streams. Part of my objection to this will be quelled if 
>>> the LLVM streams are not stateful like C++ streams are (add an extra 
>>> "std::hex" into the stream and suddenly other things down stream start 
>>> coming out in hex). As long as printf functionality is in the LLVM streams 
>>> I am ok with it.
>>> 
• Use llvm::Error instead of lldb::Error
• llvm::Error is an error class that *requires* you to 
 check whether it succeeded or it will assert.  In a way, it's similar to a 
 C++ exception, except that it doesn't come with the performance hit 
 associated with exceptions.  It's extensible, and can be easily extended 
 to support the various ways LLDB needs to construct errors and error 
 messages.
• Would need to first rename lldb::Error to LLDBError 
 so that te conversion from LLDBError to llvm::Error could be done 
 incrementally.
• Risk: 7
• Impact: 7
• Difficulty / Effort: 8
>>> 
>>> We must be very careful here. Nothing can crash LLDB and adding something 
>>> that will be known to crash in some cases can only be changed if there is a 
>>> test that can guarantee that it won't crash. assert() calls in a shared 
>>> library like LLDB are not OK and shouldn't fire. Even if they do, when 
>>> asserts are off, then it shouldn't crash LLDB. We have made efforts to only 
>>> use asserts in LLDB for things that really can't happen, but for things 
>>> like constructing a StringRef with NULL is one example of why I don't want 
>>> to use certain classes in LLVM. If we can remove the crash threat, then 
>>> lets use them. But many people firmly believe that asserts belong in llvm 
>>> and clang code and I don't agree.
>> 
>> I’m surprise by your aversion to assertions, what is your suggested 
>> alternative? Are you expecting to check and handle every possible invariants 
>> everywhere and recover (or signal an error) properly? That does not seem 
>> practical, and it seems to me that it'd lead to just involving UB (or 
>> breaking design invariant) without actually noticing it.
> 
> We have to as a debugger. We get input from a variety of different 

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

On Mon, Sep 19, 2016 at 2:37 PM Greg Clayton  wrote:

>
> > Just thinking out loud here, but one possibility is to have multiple
> pools.  One which is ref counted and one which isn't.  If you need
> something to live forever, vend it from the non-ref-counted pool.
> Otherwise vend it from the ref counted pool.
>
> Again, since we already are using LLVM under the hood here, I would hope
> to avoid changes if possible.
>
> Right, but we aren't specifically using the llvm::StringPool class which
ref counts.  idk, maybe there wouldnt' be much savings, but I remember you
saying a long time ago that ConstStrings are a huge source of memory usage
in LLDB.  So it seems like if we could ref-count when possible, it would be
a major savings.
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] LLDB Evolution - Final Form

[Enrico Granata via lldb-dev]

> On Sep 19, 2016, at 2:31 PM, Zachary Turner via lldb-dev 
>  wrote:
> 
> 
> 
> On Mon, Sep 19, 2016 at 2:20 PM Mehdi Amini  > wrote:
> 
> 
> I’m surprise by your aversion to assertions, what is your suggested 
> alternative? Are you expecting to check and handle every possible invariants 
> everywhere and recover (or signal an error) properly? That does not seem 
> practical, and it seems to me that it'd lead to just involving UB (or 
> breaking design invariant) without actually noticing it.
> 
> > Also many asserts are in header files so even if you build llvm and clang 
> > with asserts off, but then build our project that uses LLVM with asserts 
> > on, you get LLVM and clang asserts when you don't want them.
> 
> It is not really supported to use the LLVM C++ headers without assertions and 
> link to a LLVM build with assertions (and vice-versa).
>  
> It seems to me that you are mixing two things: asserting on user inputs vs 
> asserting on internal invariants of the system. LLVM is very intensive about 
> asserting on the second category and this seems fine to me.
> Asserting on external inputs is not great (in LLVM as much as in LLDB).
> 
> The asserting error class above falls into the second category and is a great 
> tool to enforce programmer error
> 
> 
> I can kind of see both sides on this one.  Yes, if you can catch instances of 
> UB before it happens that's helpful.  At the same time, when you've got a 
> product (such as, say, Xcode), you also want to try as hard as possible not 
> to bring the whole product down.  In lldb we have lldbassert, which asserts 
> for real in debug, but in release it logs an error to a file.  This is nice 
> because if someone submits a crash report, we can see the assertion failure 
> in the log file.
> 
> Now, obviously the *real* solution is to make LLDB out-of-process so it 
> doesn't bring down Xcode when it crashes.  But (and I have no knowledge of 
> the Xcode / LLDB integration) I can easily see this being a monumental amount 
> of effort.
> 

Even then, IMO you are not out of the business of not crashing on people
Sure, now you didn't crash all of Xcode, but you still lost somebody's debug 
session with potentially valuable state in it...

It sounds like llvm::Expected + lldbassert() is a good combination; it makes 
sure we don't just drop error conditions on the ground, but it also makes sure 
that we don't just give up the ghost and crash..

> Personally, asserting in LLDB doesn't affect me and I'm fine with lldb::Error 
> asserting..  But I can kind of see the argument for not wanting it to assert. 
>  
> 
> On the other hand, if we had better test coverage, then all

I wonder if catching them all can't somehow be reduced to the halting problem 
:) Most would already be great!

> of these assertions would be caught during test anyway.  Which brings us back 
> to item #2 on my list...
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Thanks,
- Enrico
 egranata@.com ☎️ 27683

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

FWIW, strlen(nullptr) will also crash just as easily as StringRef(nullptr)
will crash, so that one isn't a particularly convincing example of poorly
used asserts, since the onus on the developer is exactly the same as with
strlen.  That said, I still know where you're coming from :)

On Mon, Sep 19, 2016 at 2:34 PM Greg Clayton  wrote:

>
> > On Sep 19, 2016, at 2:20 PM, Mehdi Amini  wrote:
> >
> >>
> >> On Sep 19, 2016, at 2:02 PM, Greg Clayton via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >>
> >>
> >>> On Sep 19, 2016, at 1:18 PM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >>>
> >>> Following up with Kate's post from a few weeks ago, I think the dust
> has settled on the code reformat and it went over pretty smoothly for the
> most part.  So I thought it might be worth throwing out some ideas for
> where we go from here.  I have a large list of ideas (more ideas than time,
> sadly) that I've been collecting over the past few weeks, so I figured I
> would throw them out in the open for discussion.
> >>>
> >>> I’ve grouped the areas for improvement into 3 high level categories.
> >>>
> >>> • De-inventing the wheel - We should use more code from LLVM, and
> delete code in LLDB where LLVM provides a solution.  In cases where there
> is an LLVM thing that is *similar* to what we need, we should extend the
> LLVM thing to support what we need, and then use it.  Following are some
> areas I've identified.  This list is by no means complete.  For each one,
> I've given a personal assessment of how likely it is to cause some
> (temporary) hiccups, how much it would help us in the long run, and how
> difficult it would be to do.  Without further ado:
> >>> • Use llvm::Regex instead of lldb::Regex
> >>> • llvm::Regex doesn’t support enhanced mode.
> Could we add support for this to llvm::Regex?
> >>> • Risk: 6
> >>> • Impact: 3
> >>> • Difficulty / Effort: 3  (5 if we have to add
> enhanced mode support)
> >>
> >> As long as it supports all of the features, then this is fine.
> Otherwise we will need to modify the regular expressions to work with the
> LLVM version or back port any advances regex features we need into the LLVM
> regex parser.
> >>
> >>> • Use llvm streams instead of lldb::StreamString
> >>> • Supports output re-targeting (stderr, stdout,
> std::string, etc), printf style formatting, and type-safe streaming
> operators.
> >>> • Interoperates nicely with many existing llvm
> utility classes
> >>> • Risk: 4
> >>> • Impact: 5
> >>> • Difficulty / Effort: 7
> >>
> >> I have worked extensively with both C++ streams and with printf. I
> don't find the type safe streaming operators to be readable and a great way
> to place things into streams. Part of my objection to this will be quelled
> if the LLVM streams are not stateful like C++ streams are (add an extra
> "std::hex" into the stream and suddenly other things down stream start
> coming out in hex). As long as printf functionality is in the LLVM streams
> I am ok with it.
> >>
> >>> • Use llvm::Error instead of lldb::Error
> >>> • llvm::Error is an error class that *requires*
> you to check whether it succeeded or it will assert.  In a way, it's
> similar to a C++ exception, except that it doesn't come with the
> performance hit associated with exceptions.  It's extensible, and can be
> easily extended to support the various ways LLDB needs to construct errors
> and error messages.
> >>> • Would need to first rename lldb::Error to
> LLDBError so that te conversion from LLDBError to llvm::Error could be done
> incrementally.
> >>> • Risk: 7
> >>> • Impact: 7
> >>> • Difficulty / Effort: 8
> >>
> >> We must be very careful here. Nothing can crash LLDB and adding
> something that will be known to crash in some cases can only be changed if
> there is a test that can guarantee that it won't crash. assert() calls in a
> shared library like LLDB are not OK and shouldn't fire. Even if they do,
> when asserts are off, then it shouldn't crash LLDB. We have made efforts to
> only use asserts in LLDB for things that really can't happen, but for
> things like constructing a StringRef with NULL is one example of why I
> don't want to use certain classes in LLVM. If we can remove the crash
> threat, then lets use them. But many people firmly believe that asserts
> belong in llvm and clang code and I don't agree.
> >
> > I’m surprise by your aversion to assertions, what is your suggested
> alternative? Are you expecting to check and handle every possible
> invariants everywhere and recover (or signal an error) properly? That does
> not seem practical, and it seems to me that it'd 

Re: [lldb-dev] LLDB Evolution - Final Form

[Greg Clayton via lldb-dev]

> On Sep 19, 2016, at 2:24 PM, Zachary Turner  wrote:
> 
> 
> 
> On Mon, Sep 19, 2016 at 2:02 PM Greg Clayton  wrote:
> 
> > On Sep 19, 2016, at 1:18 PM, Zachary Turner via lldb-dev 
> >  wrote:
> >
> > Following up with Kate's post from a few weeks ago, I think the dust has 
> > settled on the code reformat and it went over pretty smoothly for the most 
> > part.  So I thought it might be worth throwing out some ideas for where we 
> > go from here.  I have a large list of ideas (more ideas than time, sadly) 
> > that I've been collecting over the past few weeks, so I figured I would 
> > throw them out in the open for discussion.
> >
> > I’ve grouped the areas for improvement into 3 high level categories.
> >
> >   • De-inventing the wheel - We should use more code from LLVM, and 
> > delete code in LLDB where LLVM provides a solution.  In cases where there 
> > is an LLVM thing that is *similar* to what we need, we should extend the 
> > LLVM thing to support what we need, and then use it.  Following are some 
> > areas I've identified.  This list is by no means complete.  For each one, 
> > I've given a personal assessment of how likely it is to cause some 
> > (temporary) hiccups, how much it would help us in the long run, and how 
> > difficult it would be to do.  Without further ado:
> >   • Use llvm::Regex instead of lldb::Regex
> >   • llvm::Regex doesn’t support enhanced mode.  Could 
> > we add support for this to llvm::Regex?
> >   • Risk: 6
> >   • Impact: 3
> >   • Difficulty / Effort: 3  (5 if we have to add 
> > enhanced mode support)
> 
> As long as it supports all of the features, then this is fine. Otherwise we 
> will need to modify the regular expressions to work with the LLVM version or 
> back port any advances regex features we need into the LLVM regex parser.
> 
> >   • Use llvm streams instead of lldb::StreamString
> >   • Supports output re-targeting (stderr, stdout, 
> > std::string, etc), printf style formatting, and type-safe streaming 
> > operators.
> >   • Interoperates nicely with many existing llvm 
> > utility classes
> >   • Risk: 4
> >   • Impact: 5
> >   • Difficulty / Effort: 7
> 
> I have worked extensively with both C++ streams and with printf. I don't find 
> the type safe streaming operators to be readable and a great way to place 
> things into streams. Part of my objection to this will be quelled if the LLVM 
> streams are not stateful like C++ streams are (add an extra "std::hex" into 
> the stream and suddenly other things down stream start coming out in hex). As 
> long as printf functionality is in the LLVM streams I am ok with it.
> I agree with you on the streams for the most part.  llvm streams do not use 
> stateful manipulators, although they do have stateless manipulators.  For 
> example, you can write:
> 
> stream << llvm::format_hex(n);
> 
> and that would print n as hex, but it wouldn't affect the printing of 
> subsequent items.  You also still get printf style formatting by using the 
> llvm::format stateless manipulator.  Like this:
> 
> stream << llvm::format("%0.4f", myfloat)
>  
> 
> >   • Use llvm::Error instead of lldb::Error
> >   • llvm::Error is an error class that *requires* you 
> > to check whether it succeeded or it will assert.  In a way, it's similar to 
> > a C++ exception, except that it doesn't come with the performance hit 
> > associated with exceptions.  It's extensible, and can be easily extended to 
> > support the various ways LLDB needs to construct errors and error messages.
> >   • Would need to first rename lldb::Error to LLDBError 
> > so that te conversion from LLDBError to llvm::Error could be done 
> > incrementally.
> >   • Risk: 7
> >   • Impact: 7
> >   • Difficulty / Effort: 8
> 
> We must be very careful here. Nothing can crash LLDB and adding something 
> that will be known to crash in some cases can only be changed if there is a 
> test that can guarantee that it won't crash. assert() calls in a shared 
> library like LLDB are not OK and shouldn't fire. Even if they do, when 
> asserts are off, then it shouldn't crash LLDB. We have made efforts to only 
> use asserts in LLDB for things that really can't happen, but for things like 
> constructing a StringRef with NULL is one example of why I don't want to use 
> certain classes in LLVM. If we can remove the crash threat, then lets use 
> them. But many people firmly believe that asserts belong in llvm and clang 
> code and I don't agree. Also many asserts are in header files so even if you 
> build llvm and clang with asserts off, but then build our project that uses 
> LLVM 

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

On Mon, Sep 19, 2016 at 2:20 PM Mehdi Amini  wrote:

>
>
> I’m surprise by your aversion to assertions, what is your suggested
> alternative? Are you expecting to check and handle every possible
> invariants everywhere and recover (or signal an error) properly? That does
> not seem practical, and it seems to me that it'd lead to just involving UB
> (or breaking design invariant) without actually noticing it.
>
> > Also many asserts are in header files so even if you build llvm and
> clang with asserts off, but then build our project that uses LLVM with
> asserts on, you get LLVM and clang asserts when you don't want them.
>
> It is not really supported to use the LLVM C++ headers without assertions
> and link to a LLVM build with assertions (and vice-versa).
>


> It seems to me that you are mixing two things: asserting on user inputs vs
> asserting on internal invariants of the system. LLVM is very intensive
> about asserting on the second category and this seems fine to me.
> Asserting on external inputs is not great (in LLVM as much as in LLDB).
>
> The asserting error class above falls into the second category and is a
> great tool to enforce programmer error
>
>
I can kind of see both sides on this one.  Yes, if you can catch instances
of UB before it happens that's helpful.  At the same time, when you've got
a product (such as, say, Xcode), you also want to try as hard as possible
not to bring the whole product down.  In lldb we have lldbassert, which
asserts for real in debug, but in release it logs an error to a file.  This
is nice because if someone submits a crash report, we can see the assertion
failure in the log file.

Now, obviously the *real* solution is to make LLDB out-of-process so it
doesn't bring down Xcode when it crashes.  But (and I have no knowledge of
the Xcode / LLDB integration) I can easily see this being a monumental
amount of effort.

Personally, asserting in LLDB doesn't affect me and I'm fine with
lldb::Error asserting..  But I can kind of see the argument for not wanting
it to assert.

On the other hand, if we had better test coverage, then all of these
assertions would be caught during test anyway.  Which brings us back to
item #2 on my list...
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] LLDB Evolution - Final Form

[Greg Clayton via lldb-dev]

> On Sep 19, 2016, at 2:11 PM, Zachary Turner  wrote:
> 
> 
> 
> On Mon, Sep 19, 2016 at 2:02 PM Greg Clayton  wrote:
> 
> >   • Separate testing tools
> >   • One question that remains open is how to represent 
> > the complicated needs of a debugger in lit tests.  Part a) above covers the 
> > trivial cases, but what about the difficult cases?  In 
> > https://reviews.llvm.org/D24591 a number of ideas were discussed.  We 
> > started getting to this idea towards the end, about a separate tool which 
> > has an interface independent of the command line interface and which can be 
> > used to test.  lldb-mi was mentioned.  While I have serious concerns about 
> > lldb-mi due to its poorly written and tested codebase, I do agree in 
> > principle with the methodology.  In fact, this is the entire philosophy 
> > behind lit as used with LLVM, clang, lld, etc.
> 
> We have a public API... Why are we going to go and spend _any_ time on doing 
> anything else? I just don't get it. What a waste of time. We have a public 
> API. Lets use it. Not simple enough for people? Tough. Testing a debugger 
> should be done using the public API except when we are actually trying to 
> test the LLDB command line in pexpect like tests. Those and only those are 
> fine to covert over to using LIT and use text scraping. But 95% of our 
> testing should be done using the API that our IDEs use. Using MI? Please no.
> 
> FWIW, I agree with you about mi.
> 
> That said, the public api is problematic.  Here you've got an API which, once 
> you do something to, is set in stone forever.  And yet in order to test 
> something, you have to add it to the API.  So now, in order to test 
> something, you have to make a permanent change to a public API that can never 
> be undone.

So let me clarify: I don't want people adding to the public API for the sole 
reason of testing.. That falls into the "test another way" category. So I 
should have said "add it to the public API if it would be a valid addition to 
the public API.
> 
> It's also a public facing API, when a lot of the stuff  we need to be able to 
> look at and inspect is not really suitable for public consumption.

Again, this falls into the "test another way" category as is perfect for gtest 
or lit.
> 
> If something is a public API that can never be changed once it's added, then 
> there should be an extremely strict review process in order to add something 
> to it.  It should be VERY HARD to modify (it's currently not, which is a 
> debate for another day, but anyway...).  And yet that's fundamentally 
> incompatible with having tests be easy to write.  When it's hard to add the 
> thing you need to add in order to write the test, then it's hard to write the 
> test.

Yes yes yes. Sorry about the confusion. My first sentence should clear this 
up...

> 
> Furthermore, with a system such as the one I proposed, you could even run 
> tests with LLDB_DISABLE_PYTHON.  IDK about you, but that would be *amazing* 
> from my point of view.  Even though I spent months making Python 3 work, I 
> would be happy to burn it all with fire and go back to just Python 2 if we 
> had a viable testing strategy.

I am not opposed to that. That being say we could start having most API tests 
actually use the C++ public API and have the tests written in C++. We would 
need a .a file with some canned functionality in it, but anything that is 
testing the public API could just be C++. 

So I definitely don't want people adding to the public API just for testing. 
The rules should be:
- Does my feature belong in the public API?
  - Yes, great, add it and test it there
  - No, test it with gtest or lit

Greg


___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

On Mon, Sep 19, 2016 at 2:02 PM Greg Clayton  wrote:

>
> > On Sep 19, 2016, at 1:18 PM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >
> > Following up with Kate's post from a few weeks ago, I think the dust has
> settled on the code reformat and it went over pretty smoothly for the most
> part.  So I thought it might be worth throwing out some ideas for where we
> go from here.  I have a large list of ideas (more ideas than time, sadly)
> that I've been collecting over the past few weeks, so I figured I would
> throw them out in the open for discussion.
> >
> > I’ve grouped the areas for improvement into 3 high level categories.
> >
> >   • De-inventing the wheel - We should use more code from LLVM, and
> delete code in LLDB where LLVM provides a solution.  In cases where there
> is an LLVM thing that is *similar* to what we need, we should extend the
> LLVM thing to support what we need, and then use it.  Following are some
> areas I've identified.  This list is by no means complete.  For each one,
> I've given a personal assessment of how likely it is to cause some
> (temporary) hiccups, how much it would help us in the long run, and how
> difficult it would be to do.  Without further ado:
> >   • Use llvm::Regex instead of lldb::Regex
> >   • llvm::Regex doesn’t support enhanced mode.
> Could we add support for this to llvm::Regex?
> >   • Risk: 6
> >   • Impact: 3
> >   • Difficulty / Effort: 3  (5 if we have to add
> enhanced mode support)
>
> As long as it supports all of the features, then this is fine. Otherwise
> we will need to modify the regular expressions to work with the LLVM
> version or back port any advances regex features we need into the LLVM
> regex parser.
>
> >   • Use llvm streams instead of lldb::StreamString
> >   • Supports output re-targeting (stderr, stdout,
> std::string, etc), printf style formatting, and type-safe streaming
> operators.
> >   • Interoperates nicely with many existing llvm
> utility classes
> >   • Risk: 4
> >   • Impact: 5
> >   • Difficulty / Effort: 7
>
> I have worked extensively with both C++ streams and with printf. I don't
> find the type safe streaming operators to be readable and a great way to
> place things into streams. Part of my objection to this will be quelled if
> the LLVM streams are not stateful like C++ streams are (add an extra
> "std::hex" into the stream and suddenly other things down stream start
> coming out in hex). As long as printf functionality is in the LLVM streams
> I am ok with it.
>
I agree with you on the streams for the most part.  llvm streams do not use
stateful manipulators, although they do have stateless manipulators.  For
example, you can write:

stream << llvm::format_hex(n);

and that would print n as hex, but it wouldn't affect the printing of
subsequent items.  You also still get printf style formatting by using the
llvm::format stateless manipulator.  Like this:

stream << llvm::format("%0.4f", myfloat)


>
> >   • Use llvm::Error instead of lldb::Error
> >   • llvm::Error is an error class that *requires*
> you to check whether it succeeded or it will assert.  In a way, it's
> similar to a C++ exception, except that it doesn't come with the
> performance hit associated with exceptions.  It's extensible, and can be
> easily extended to support the various ways LLDB needs to construct errors
> and error messages.
> >   • Would need to first rename lldb::Error to
> LLDBError so that te conversion from LLDBError to llvm::Error could be done
> incrementally.
> >   • Risk: 7
> >   • Impact: 7
> >   • Difficulty / Effort: 8
>
> We must be very careful here. Nothing can crash LLDB and adding something
> that will be known to crash in some cases can only be changed if there is a
> test that can guarantee that it won't crash. assert() calls in a shared
> library like LLDB are not OK and shouldn't fire. Even if they do, when
> asserts are off, then it shouldn't crash LLDB. We have made efforts to only
> use asserts in LLDB for things that really can't happen, but for things
> like constructing a StringRef with NULL is one example of why I don't want
> to use certain classes in LLVM. If we can remove the crash threat, then
> lets use them. But many people firmly believe that asserts belong in llvm
> and clang code and I don't agree. Also many asserts are in header files so
> even if you build llvm and clang with asserts off, but then build our
> project that uses LLVM with asserts on, you get LLVM and clang asserts when
> you don't want them.
>
We can probably add something to llvm::Error to make it not assert but to
do something else instead.  Something that would be 

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

On Mon, Sep 19, 2016 at 2:02 PM Greg Clayton  wrote:

>
> >   • Separate testing tools
> >   • One question that remains open is how to
> represent the complicated needs of a debugger in lit tests.  Part a) above
> covers the trivial cases, but what about the difficult cases?  In
> https://reviews.llvm.org/D24591 a number of ideas were discussed.  We
> started getting to this idea towards the end, about a separate tool which
> has an interface independent of the command line interface and which can be
> used to test.  lldb-mi was mentioned.  While I have serious concerns about
> lldb-mi due to its poorly written and tested codebase, I do agree in
> principle with the methodology.  In fact, this is the entire philosophy
> behind lit as used with LLVM, clang, lld, etc.
>
> We have a public API... Why are we going to go and spend _any_ time on
> doing anything else? I just don't get it. What a waste of time. We have a
> public API. Lets use it. Not simple enough for people? Tough. Testing a
> debugger should be done using the public API except when we are actually
> trying to test the LLDB command line in pexpect like tests. Those and only
> those are fine to covert over to using LIT and use text scraping. But 95%
> of our testing should be done using the API that our IDEs use. Using MI?
> Please no.


FWIW, I agree with you about mi.

That said, the public api is problematic.  Here you've got an API which,
once you do something to, is set in stone forever.  And yet in order to
test something, you have to add it to the API.  So now, in order to test
something, you have to make a permanent change to a public API that can
never be undone.

It's also a public facing API, when a lot of the stuff  we need to be able
to look at and inspect is not really suitable for public consumption.

If something is a public API that can never be changed once it's added,
then there should be an extremely strict review process in order to add
something to it.  It should be VERY HARD to modify (it's currently not,
which is a debate for another day, but anyway...).  And yet that's
fundamentally incompatible with having tests be easy to write.  When it's
hard to add the thing you need to add in order to write the test, then it's
hard to write the test.

Furthermore, with a system such as the one I proposed, you could even run
tests with LLDB_DISABLE_PYTHON.  IDK about you, but that would be *amazing*
from my point of view.  Even though I spent months making Python 3 work, I
would be happy to burn it all with fire and go back to just Python 2 if we
had a viable testing strategy.
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

On Mon, Sep 19, 2016 at 1:57 PM Enrico Granata  wrote:

>
> I am definitely not innocent in this regard. However, it does happen for a
> reason.
>
> Sometimes, I am writing code in lldb that is the foundation of something I
> need to do over on the Swift.org side.
>
> I'll lay out foundational work/groundwork/plumbing code/... on the
> llvm.org side of the fence, but that code is a no-op there. The real
> grunt work happens on the Swift support. It's just architecturally sound to
> have non-Swift-specific bits happen on the llvm.org side. When that
> happens, I have no reasonable way (in the current model) to test the
> groundwork - it's just an intricate no-op that doesn't get activated.
>
> There are tests. They are on a different repo. It's not great, I'll admit.
> But right now, I would have to design an API around those bits even though
> I don't need one, or add commands I don't want "just" for testing. That is
> polluting a valuable user-facing resource with implementation details. I
> would gladly jump on a testing infrastructure that lets me write tests for
> this kind of code without extra API/commands.
>

Part of the problem is just that I think we don't have the tools we need to
write effective tests.  We have a lot of tests that only work on a
particular platform, or with a particular language runtime, or all these
different variables that affect the configurations under which the test
should pass.  In an ideal world we would be able to test the lion's share
of the debugger on any platform.  The only thing that's platform specific
is really the loader.  But maybe we have a huge block of code that itself
is not platform specific, but only gets run as a child of some platform
specific branch.  Now coverage of that platform-inedpenent branch is
limited or non-existant, because it depends on being able to have juuust
the right setup to tickle the debugger into running it.

So I just want to re-iterate that I don't think anyone is to blame, we just
don't have the right tools that we need.  I think the separate tools that I
mentioned could go a long way to rectifying that, because you can skip past
all of the platform specific aspects and test the code that runs behind
them.

Of course, you'd still have your full integration tests, but you'd now have
much fewer.  And when they fail, you'd have a pretty good idea where to
look because presumably the failure would be specific to some bit of
platform specific functionality.

I remmeber one case where TestUnsigned failed on Windows but TestSigned
passed.  And when we ultimately tracked down the error, it had absolutely
nothing to do with signed-ness.  Because there were too many layers in
between the test and the functionality being tested.
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] LLDB Evolution - Final Form

[Enrico Granata via lldb-dev]

A few remarks

> On Sep 19, 2016, at 1:18 PM, Zachary Turner via lldb-dev 
>  wrote:
> 
> Following up with Kate's post from a few weeks ago, I think the dust has 
> settled on the code reformat and it went over pretty smoothly for the most 
> part.  So I thought it might be worth throwing out some ideas for where we go 
> from here.  I have a large list of ideas (more ideas than time, sadly) that 
> I've been collecting over the past few weeks, so I figured I would throw them 
> out in the open for discussion.
> 
> I’ve grouped the areas for improvement into 3 high level categories.
> 
> De-inventing the wheel - We should use more code from LLVM, and delete code 
> in LLDB where LLVM provides a solution.  In cases where there is an LLVM 
> thing that is *similar* to what we need, we should extend the LLVM thing to 
> support what we need, and then use it.  Following are some areas I've 
> identified.  This list is by no means complete.  For each one, I've given a 
> personal assessment of how likely it is to cause some (temporary) hiccups, 
> how much it would help us in the long run, and how difficult it would be to 
> do.  Without further ado:
> Use llvm::Regex instead of lldb::Regex
> llvm::Regex doesn’t support enhanced mode.  Could we add support for this to 
> llvm::Regex?
> Risk: 6
> Impact: 3
> Difficulty / Effort: 3  (5 if we have to add enhanced mode support)
> Use llvm streams instead of lldb::StreamString
> Supports output re-targeting (stderr, stdout, std::string, etc), printf style 
> formatting, and type-safe streaming operators.
> Interoperates nicely with many existing llvm utility classes
> Risk: 4
> Impact: 5
> Difficulty / Effort: 7
> Use llvm::Error instead of lldb::Error
> llvm::Error is an error class that *requires* you to check whether it 
> succeeded or it will assert.

I assume that assertion would be stripped in Release builds?
We have our own lldbassert() macro currently, which assert()s in Debug mode, 
but in Release mode produces an error message and continues
It would be great if llvm::Error allowed us to plug that in..

>   In a way, it's similar to a C++ exception, except that it doesn't come with 
> the performance hit associated with exceptions.  It's extensible, and can be 
> easily extended to support the various ways LLDB needs to construct errors 
> and error messages.
> Would need to first rename lldb::Error to LLDBError so that te conversion 
> from LLDBError to llvm::Error could be done incrementally.
> Risk: 7
> Impact: 7
> Difficulty / Effort: 8
> StringRef instead of const char *, len everywhere
> Can do most common string operations in a way that is guaranteed to be safe.
> Reduces string manipulation algorithm complexity by an order of magnitude.
> Can potentially eliminate tens of thousands of string copies across the 
> codebase.
> Simplifies code.
> Risk: 3
> Impact: 8
> Difficulty / Effort: 7
> ArrayRef instead of const void *, len everywhere
> Same analysis as StringRef
> MutableArrayRef instead of void *, len everywhere
> Same analysis as StringRef

I don't think we have a lot of those - IIRC, it's mostly in the SB API where 
SWIG is supposed to map it back to a Python string

> Delete ConstString, use a modified StringPool that is thread-safe.
> StringPool is a non thread-safe version of ConstString.
> Strings are internally refcounted so they can be cleaned up when they are no 
> longer used.  ConstStrings are a large source of memory in LLDB, so 
> ref-counting and removing stale strings has the potential to be a huge 
> savings.
> Risk: 2
> Impact: 9
> Difficulty / Effort: 6
> thread_local instead of lldb::ThreadLocal
> This fixes a number of bugs on Windows that cannot be fixed otherwise, as 
> they require compiler support.
> Some other compilers may not support this yet?
> Risk: 2
> Impact: 3
> Difficulty: 3
> Use llvm::cl for the command line arguments to the primary lldb executable.
> Risk: 2
> Impact: 3
> Difficulty / Effort: 4
> Testing - Our testing infrastructure is unstable, and our test coverage is 
> lacking.  We should take steps to improve this.
> Port as much as possible to lit
> Simple tests should be trivial to port to lit today.  If nothing else this 
> serves as a proof of concept while increasing the speed and stability of the 
> test suite, since lit is a more stable harness.
> Separate testing tools
> One question that remains open is how to represent the complicated needs of a 
> debugger in lit tests.  Part a) above covers the trivial cases, but what 
> about the difficult cases?  In https://reviews.llvm.org/D24591 
>  a number of ideas were discussed.  We 
> started getting to this idea towards the end, about a separate tool which has 
> an interface independent of the command line interface and which can be used 
> to test.  lldb-mi was mentioned.  While I have serious concerns about lldb-mi 
> due to its poorly written and tested codebase, I do agree in principle 

Re: [lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

On Mon, Sep 19, 2016 at 1:38 PM Sean Callanan  wrote:

> I'll only comment on the stuff that affects me.
>
>
>1. Use llvm streams instead of lldb::StreamString
>   1. Supports output re-targeting (stderr, stdout, std::string, etc),
>  printf style formatting, and type-safe streaming operators.
>  2. Interoperates nicely with many existing llvm utility classes
>  3. Risk: 4
>  4. Impact: 5
>  5. Difficulty / Effort: 7
>
>
> I don't like that llvm's stringstream needs to be babied to make it
> produce its string.  You have to wrap it and then flush it and then read
> the string.  Maybe a subclass could be made that wraps its own string and
> flushes automatically on read?
>

You do have to wrap it.  But if you call llvm::raw_string_ostream::str(),
it will internally flush before it returns the thing.  So if you write:

std::string s;
llvm::raw_string_ostream stream(s);
stream << "foo";
return stream.str();

then the code will be correct.  It's still a bit more annoying then if the
string were automatically part of the stream though, so there's probably a
clean way to design a version of it that owns the string.
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

[lldb-dev] LLDB Evolution - Final Form

[Zachary Turner via lldb-dev]

Following up with Kate's post from a few weeks ago, I think the dust has
settled on the code reformat and it went over pretty smoothly for the most
part.  So I thought it might be worth throwing out some ideas for where we
go from here.  I have a large list of ideas (more ideas than time, sadly)
that I've been collecting over the past few weeks, so I figured I would
throw them out in the open for discussion.

I’ve grouped the areas for improvement into 3 high level categories.


   1.

   De-inventing the wheel - We should use more code from LLVM, and delete
   code in LLDB where LLVM provides a solution. In cases where there is an
   LLVM thing that is *similar* to what we need, we should extend the LLVM
   thing to support what we need, and then use it. Following are some areas
   I've identified. This list is by no means complete. For each one, I've
   given a personal assessment of how likely it is to cause some (temporary)
   hiccups, how much it would help us in the long run, and how difficult it
   would be to do. Without further ado:
   1.

  Use llvm::Regex instead of lldb::Regex
  1.

 llvm::Regex doesn’t support enhanced mode.  Could we add support
 for this to llvm::Regex?
 2.

 Risk: 6
 3.

 Impact: 3
 4.

 Difficulty / Effort: 3 (5 if we have to add enhanced mode support)
 2.

  Use llvm streams instead of lldb::StreamString
  1.

 Supports output re-targeting (stderr, stdout, std::string, etc),
 printf style formatting, and type-safe streaming operators.
 2.

 Interoperates nicely with many existing llvm utility classes
 3.

 Risk: 4
 4.

 Impact: 5
 5.

 Difficulty / Effort: 7
 3.

  Use llvm::Error instead of lldb::Error
  1.

 llvm::Error is an error class that *requires* you to check whether
 it succeeded or it will assert. In a way, it's similar to a
C++ exception,
 except that it doesn't come with the performance hit associated with
 exceptions. It's extensible, and can be easily extended to support the
 various ways LLDB needs to construct errors and error messages.
 2.

 Would need to first rename lldb::Error to LLDBError so that te
 conversion from LLDBError to llvm::Error could be done
 incrementally.
 3.

 Risk: 7
 4.

 Impact: 7
 5.

 Difficulty / Effort: 8
 4.

  StringRef instead of const char *, len everywhere
  1.

 Can do most common string operations in a way that is guaranteed
 to be safe.
 2.

 Reduces string manipulation algorithm complexity by an order of
 magnitude.
 3.

 Can potentially eliminate tens of thousands of string copies
 across the codebase.
 4.

 Simplifies code.
 5.

 Risk: 3
 6.

 Impact: 8
 7.

 Difficulty / Effort: 7
 5.

  ArrayRef instead of const void *, len everywhere
  1.

 Same analysis as StringRef
 6.

  MutableArrayRef instead of void *, len everywhere
  1.

 Same analysis as StringRef
 7.

  Delete ConstString, use a modified StringPool that is thread-safe.
  1.

 StringPool is a non thread-safe version of ConstString.
 2.

 Strings are internally refcounted so they can be cleaned up when
 they are no longer used.  ConstStrings are a large source of
 memory in LLDB, so ref-counting and removing stale strings has the
 potential to be a huge savings.
 3.

 Risk: 2
 4.

 Impact: 9
 5.

 Difficulty / Effort: 6
 8.

  thread_local instead of lldb::ThreadLocal
  1.

 This fixes a number of bugs on Windows that cannot be fixed
 otherwise, as they require compiler support.
 2.

 Some other compilers may not support this yet?
 3.

 Risk: 2
 4.

 Impact: 3
 5.

 Difficulty: 3
 9.

  Use llvm::cl for the command line arguments to the primary lldb
  executable.
  1.

 Risk: 2
 2.

 Impact: 3
 3.

 Difficulty / Effort: 4
 2.

   Testing - Our testing infrastructure is unstable, and our test coverage
   is lacking. We should take steps to improve this.
   1.

  Port as much as possible to lit
  1.

 Simple tests should be trivial to port to lit today.  If nothing
 else this serves as a proof of concept while increasing the speed and
 stability of the test suite, since lit is a more stable harness.
 2.

  Separate testing tools
  1.

 One question that remains open is how to represent the complicated
 needs of a debugger in lit tests.