[polly] r238090 - Use unique_ptr to clarify ownership of ScopStmt
tobias at grosser.es
Tue May 26 05:32:33 PDT 2015
On 05/23/2015 12:19 PM, Johannes Doerfert wrote:
> On 05/23, Tobias Grosser wrote:
>> Author: grosser
>> Date: Sat May 23 00:14:09 2015
>> New Revision: 238090
>> URL: http://llvm.org/viewvc/llvm-project?rev=238090&view=rev
>> Use unique_ptr to clarify ownership of ScopStmt
>> - typedef std::vector<ScopStmt *> StmtSet;
>> + typedef std::vector<std::unique_ptr<ScopStmt>> StmtSet;
> I'm not sure about this. Why/Where do you need clarification for the
This patch was a first mini-step of a general push towards 'no delete'
in our source code. The general practice in C++11 code seems to be:
"Always use the standard smart pointers, and non-owning raw pointers.
Never use owning raw pointers and delete, except in rare cases when
implementing your own low-level data structure (and even then keep that
well encapsulated inside a class boundary)." 
David Blaike has been working since several months on moving LLVM into
this direction and I wanted to start this effort on the Polly side as well.
If this is in any sense controversial (please tell me), I am happy to
back this out and have a discussion to reconsider this goal.
My personal motivation are some memory issues I have (in some
prototype), which would benefit from more memory safety in general (but
most likely not from this specific commit).
>> SmallPtrSet<const Value *, 8> ReductionBaseValues;
>> - for (ScopStmt *Stmt : S)
>> + for (auto &Stmt : S)
> I liked the * in the type better as you knew what you get. Now I can see
> only a reference which I usually do not suspected to be a capsuled pointer.
I agree, the original code was slightly more readable. On the other
hand, this iterator clarifies that we indeed only get a reference (and
not a separate copy) of the statement. In commit 238090, a similar
change helped me to realize that we did an unnecessary copy of a
std::pair which could just have been a reference in the range loop.
I don't feel super strong about this (it worked OK until now), but my
general feeling is that using the C++11 managed pointers will be
generally beneficial. I am happy to year opinions on this.
More information about the llvm-commits