jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.
This looks fine. There was one place where you referred back to case 1 by name
not ordinal, but otherwise this is quite clear. Don't need another round of
review, just fix that nit and its
stella.stamenova added a comment.
In https://reviews.llvm.org/D49980#1208096, @aleksandr.urakov wrote:
> It's also possible that the test has been failing since some commit in
> another repository (e.g. `clang`, `lld` or `llvm`). It seems that it was ok
> several days ago...
I think you are r
shafik added a comment.
@jingham @vsk I believe I have addressed most of your comments
Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:58
+
+ if (process != nullptr) {
+Status status;
jingham wrote:
> vsk wrote:
> > Please use an early exit here,
shafik updated this revision to Diff 161837.
shafik marked 12 inline comments as done.
shafik added a comment.
Fixes based on comments
- Switched to early exits
- Added a lot more comments to explain all the cases being dealt with and
noting which cases different sections are dealing with
http
jingham added a comment.
m_last_natural_stop_id is the stop ID for the last time we stopped when the
user was just running the process (run, continue, next, etc...)
m_last_user_expression_resume is the resume ID for the last user expression
resume.
The stops and resumes always occur in pairs i
teemperor added inline comments.
Comment at: include/lldb/Target/Process.h:435
+ bool IsRunningUtilityFunction() const {
+return m_last_natural_stop_id != m_stop_id;
+ }
@jingham That might be wrong, but I'm not sure what exactly each member
variable is tr
teemperor updated this revision to Diff 161829.
teemperor retitled this revision from "Don't cancel the current IOHandler when
we push the ProcessIO handler." to "Don't cancel the current IOHandler when we
push a handler for an utility function run.".
teemperor edited the summary of this revision
aleksandr.urakov added a comment.
It's also possible that the test has been failing since some commit in another
repository (e.g. `clang`, `lld` or `llvm`). It seems that it was ok several
days ago...
Repository:
rL LLVM
https://reviews.llvm.org/D49980
___
aleksandr.urakov added a comment.
Yes, I've seen today that this test is failing now, but if I'm not mistaken
this have happened some later than this patch was committed. But I'm not sure,
I'll look at this more precisely tomorrow, thanks!
Repository:
rL LLVM
https://reviews.llvm.org/D49980
stella.stamenova added a comment.
I am not 100% sure yet, but this change is the most likely culprit for one of
the SymbolFile tests to start failing, namely:
SymbolFile/DWARF/dwarf5-index-is-used.cpp. This fails for us on both Windows
and Linux with the following:
FAIL: lldb :: SymbolFile/D
labath accepted this revision.
labath added a comment.
Thank you for writing the test. This is more-or-less what I had in mind. Just
make sure we don't put the test files in /tmp (e.g., because windows doesn't
have it).
It's unfortunate that the core file has my username embedded in it (I shoul
jingham added a comment.
Sounds like std::function objects need to run static initializers before they
are useful. So trying to format them w/o a process won't do any good.
Probably better to just return false directly if you don't have a process in
that case.
https://reviews.llvm.org/D508
Author: adrian
Date: Tue Aug 21 09:13:37 2018
New Revision: 340293
URL: http://llvm.org/viewvc/llvm-project?rev=340293&view=rev
Log:
lldbtest.py: Unconditionally set the clang module cache path.
This should fix the errors observable on the new lldb-cmake bot.
Modified:
lldb/trunk/packages/Py
Author: adrian
Date: Tue Aug 21 08:46:15 2018
New Revision: 340286
URL: http://llvm.org/viewvc/llvm-project?rev=340286&view=rev
Log:
Makefile.rules: Use an absolute path to the module cache directory.
This change is NFC, but it makes it more obvious in log files what happened.
Modified:
lldb
14 matches
Mail list logo