This revision was automatically updated to reflect the committed changes.
Closed by commit rL273547: Add MemoryRegionInfo to SB API (authored by
hhellyer).
Changed prior to commit:
http://reviews.llvm.org/D20565?vs=61253&id=61648#toc
Repository:
rL LLVM
http://reviews.llvm.org/D20565
Files
hhellyer added a comment.
I re-updated the patch via arc diff (rather than a manual patch upload).
Phabricator now seems to know which revision my changes are based off and has
added the "Next Step: arc commit" status. Do I need to request an svn password
to run arc commit or can you (or someon
hhellyer updated this revision to Diff 61253.
hhellyer added a comment.
Updating to ensure I have the latest changes.
http://reviews.llvm.org/D20565
Files:
include/lldb/API/LLDB.h
include/lldb/API/SBDefines.h
include/lldb/API/SBMemoryRegionInfo.h
include/lldb/API/SBMemoryRegionInfoList.
hhellyer marked 12 inline comments as done.
hhellyer added a comment.
http://reviews.llvm.org/D20565
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
hhellyer added a comment.
Should I be able to deliver these changes now? When I try following the
instructions here: http://llvm.org/docs/Phabricator.html the patch
downloads and applies correctly but whether I use arc via the git or svn
commit methods I'm ultimately prompted for a password fo
hhellyer updated this revision to Diff 60517.
hhellyer added a comment.
Updated to rebase on the latest changes to MemoryRegionInfo.h
http://reviews.llvm.org/D20565
Files:
include/lldb/API/LLDB.h
include/lldb/API/SBDefines.h
include/lldb/API/SBMemoryRegionInfo.h
include/lldb/API/SBMemor
hhellyer updated this revision to Diff 59524.
hhellyer marked an inline comment as done.
hhellyer added a comment.
Fix the latest code review comments to remove unnecessary tests for a valid
pointer that will always be initialised.
The change to the == operator on SBMemoryRegionInfo required an
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
So a bit more cleanup. We should always have a valid object inside of a
SBMemoryRegionInfo or SBMemoryRegionInfoList so there is no need to ever check
the m_opaque_ap to see if i
hhellyer updated this revision to Diff 59352.
hhellyer added a comment.
Made sure an object was always constructed for the std::unique_ptr in
SBMemoryRegionInfo.cpp.
Removed unused constructor from SBMemoryRegionInfo.h.
http://reviews.llvm.org/D20565
Files:
include/lldb/API/LLDB.h
include/
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
One last thing: we should default construct a MemoryRegionInfo in all
SBMemoryRegionInfo::SBMemoryRegionInfo() constructors so we don't have a make
sure to call a non-const ref()
hhellyer added a comment.
In http://reviews.llvm.org/D20565#444507, @clayborg wrote:
> My main reason for making sure we use std::unique_ptr is for API
> compatibility in the future. The size of std::unique_ptr and std::shared_ptr
> differs and if anyone wrote an IDE or tool that links against
hhellyer updated this revision to Diff 59205.
hhellyer added a comment.
I've switch to a unique_ptr from a shared_ptr in SBMemoryRegionInfo.
http://reviews.llvm.org/D20565
Files:
include/lldb/API/LLDB.h
include/lldb/API/SBDefines.h
include/lldb/API/SBMemoryRegionInfo.h
include/lldb/API/
clayborg added a comment.
In http://reviews.llvm.org/D20565#444223, @hhellyer wrote:
> In http://reviews.llvm.org/D20565#442373, @clayborg wrote:
>
> > This looks good as long as SBMemoryRegionInfo and SBMemoryRegionInfoList
> > are ready only and will never have any setter functions. If we plan
hhellyer marked 2 inline comments as done.
hhellyer added a comment.
In http://reviews.llvm.org/D20565#442373, @clayborg wrote:
> This looks good as long as SBMemoryRegionInfo and SBMemoryRegionInfoList are
> ready only and will never have any setter functions. If we plan to ever have
> the SBM
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
This looks good as long as SBMemoryRegionInfo and SBMemoryRegionInfoList are
ready only and will never have any setter functions. If we plan to ever have
the SBMemoryRegionInfo o
hhellyer updated this revision to Diff 58771.
hhellyer added a comment.
Removed std::enable_shared_from_this from MemoryRegionInfo.h (which removes the
only change from that file)
Renamed region_info to region_info_sp in SBProcess::GetMemoryRegionInfo in
SBProcess.cpp
I also corrected the positi
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
Few minor changes, but looks great overall. Thanks for doing this.
Comment at: include/lldb/Target/MemoryRegionInfo.h:18-19
@@ -17,3 +17,4 @@
{
-class Memor
17 matches
Mail list logo
