- User Since
- Jun 4 2013, 6:02 AM (246 w, 6 h)
Seems straight-forward enough, but technically Jim is the owner of the expression evaluator these days, so I'll leave the honours to him.
How would you feel about replacing the custom debug printfs with standard logging messages?
Sat, Feb 17
Yeah, setting this to zero would break other platforms that are able to locate shared libraries before running the executable. On linux, we try to locate them as well, but it's kind of on a best-effort basis -- it's quite hard to figure out what library will get loaded with absolute precision, as it can e.g. depend on the value of LD_LIBRARY_PATH env var that you run the process with (and you don't know that until the actual "process launch" command). In fact, it wouldn't be hard to come up with examples where this static resolution finds the wrong library.
Fri, Feb 16
Remove the detail namespace, and rename the EmissionContext class.
It looks like we'll do something like D43333 instead...
I've updated the format to include a description of the item. I don't we need to
worry about the expensiveness of this data structure too much, as it should
never be used in a hot loop (the only use case I can think of is querying the
configuration at start-up).
Some nits, but nothing major from me.
Thu, Feb 15
It doesn't look like there is. Do you need someone to check this in?
This is unfortunate, but it's kind of a consequence of how googletest implements it's universal printers. The reason why I don't think it's a good idea is that this is not a problem specific to StringRef, as any class can come out as giberrish if the user forgets to include the PrintTo function. I am not adamant about this, but I think the bar for modifying gtest code should be higher than this (it's trivial to add the right include if you see you're values aren't coming out right).
This code does not have to live directly inside gtest. As such, maybe a better place for it would be somewhere under include/llvm/Testing ? (We already have PrintTo functions for Error and Expected there..)
Wed, Feb 14
Make base Entry destructor protected and nonvirtual (the class is not owned polymorphically right now).
delete copy constructors of AccelTableBase and move one comment a bit.
I've also put up D43286 (which is still WIP, but nearing completion...) so you can see the context of this change.
This is now superseeded by D43286.
Mon, Feb 12
Make the base class copy/move operations protected and remote the hand-coded implementations from the derived class.
(Btw, if you're looking for things to FileCheck-ify, I think the stuff under lldb/unittests/UnwindAssembly is a prime candidate and has a much higher bang/buck ratio.)
There are several components in the "command line parsing"
- argparse: parsing a single string into the individual words (processing quotes, backslashes, etc.)
- resolving the "command" part of the command line (the "frame variable" part of "frame variable --raw-output FOO")
- resolving the arguments of a command (the "--raw-output FOO" part)
Fri, Feb 9
Resolve David's comments (apart from one, which I have a question on).
Modify the code generator script to produce patterns suggested by joerg. This
will make the function return quickly for lower-valued characters, which are
probably more likely to appear in input.
Thu, Feb 8
Fix issues pointed out by Jonas + clang-format.
The change seems fine to me, and I don't really have anything to add to the things that were said already.
I believe I have resolved all outstanding issues. If there's anything else you'd like me to do, let me know (this is mainly addressed at @probinson, as his initial NACK is keeping this review red).
That seems extremely dodgy. Surely whether something is a mangled name does not depend on the compiler we built lldb with. I am aware that we have many places with _Z hardcoded, but this seems to be making the situation only worse.
Wed, Feb 7
I'm confused here. Can you share the exact commands you use to setup the debug session?
Tue, Feb 6
Looks good, however I should point out that the state of the lldb+python3+linux combo is unknown..
Heh, no good deed goes unpunished, right.. :)
As I mentioned in the patch above, I don't think it's worth it trying to tiptoe here, as for 99% of files (basically, anything that is not yaml2obj's output, I think), we will end up reading the whole file anyway. It just increases the number of things that can go wrong.
This looks much better. Thanks.
Being more resilient when handling demangler outputs seems like a good thing, but I think it is important to understand what made the demangler produce that output in the first place, to make sure we aren't missing anything.
Mon, Feb 5
> I think that a more reasonable behavior here would be to copy the file contents instead.
Extending lldb-test's dumpModules() to also dump out the module's triple sounds like a reasonable thing to do (I am assuming that the Module class will do something reasonable when given an object file with no sections, like a core file -- if not we could make a separate command for dumping this out).