<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Mar 31, 2016, at 4:35 PM, Rui Ueyama <<a href="mailto:ruiu@google.com" class="">ruiu@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_extra"><div class="gmail_quote">On Thu, Mar 31, 2016 at 4:18 PM, Mehdi Amini<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:mehdi.amini@apple.com" target="_blank" class="">mehdi.amini@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;" class=""><br class=""><div class=""><div class=""><div class="h5"><blockquote type="cite" class=""><div class="">On Mar 31, 2016, at 4:14 PM, Rui Ueyama <<a href="mailto:ruiu@google.com" target="_blank" class="">ruiu@google.com</a>> wrote:</div><br class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;" class=""><div class="gmail_extra"><div class="gmail_quote">On Thu, Mar 31, 2016 at 4:05 PM, Mehdi Amini via llvm-commits<span class=""> </span><span dir="ltr" class=""><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>></span><span class=""> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;">Author: mehdi_amini<br class="">Date: Thu Mar 31 18:05:26 2016<br class="">New Revision: 265068<br class=""><br class="">URL:<span class=""> </span><a href="http://llvm.org/viewvc/llvm-project?rev=265068&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=265068&view=rev</a><br class="">Log:<br class="">Add disk_space() to llvm::fs<br class=""><br class="">Summary: Adapted from Boost::filesystem.<br class="">(This is a reapply by reverting commit r265062 and fixing the WinAPI part)<br class=""><br class="">Differential Revision:<span class=""> </span><a href="http://reviews.llvm.org/D18467" rel="noreferrer" target="_blank" class="">http://reviews.llvm.org/D18467</a><br class=""><br class="">From: Mehdi Amini <<a href="mailto:mehdi.amini@apple.com" target="_blank" class="">mehdi.amini@apple.com</a>><br class=""><br class="">Modified:<br class=""> <span class=""> </span>llvm/trunk/include/llvm/Support/FileSystem.h<br class=""> <span class=""> </span>llvm/trunk/lib/Support/Unix/Path.inc<br class=""> <span class=""> </span>llvm/trunk/lib/Support/Windows/Path.inc<br class=""><br class="">Modified: llvm/trunk/include/llvm/Support/FileSystem.h<br class="">URL:<span class=""> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FileSystem.h?rev=265068&r1=265067&r2=265068&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FileSystem.h?rev=265068&r1=265067&r2=265068&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/include/llvm/Support/FileSystem.h (original)<br class="">+++ llvm/trunk/include/llvm/Support/FileSystem.h Thu Mar 31 18:05:26 2016<br class="">@@ -32,6 +32,7 @@<br class=""> #include "llvm/ADT/Twine.h"<br class=""> #include "llvm/Support/DataTypes.h"<br class=""> #include "llvm/Support/ErrorHandling.h"<br class="">+#include "llvm/Support/ErrorOr.h"<br class=""> #include "llvm/Support/TimeValue.h"<br class=""> #include <ctime><br class=""> #include <iterator><br class="">@@ -648,6 +649,17 @@ std::error_code identify_magic(const Twi<br class=""><br class=""> std::error_code getUniqueID(const Twine Path, UniqueID &Result);<br class=""><br class="">+/// @brief Get disk space usage information.<br class="">+///<br class="">+/// Note: Users must be careful about "Time Of Check, Time Of Use" kind of bug.<br class="">+/// Note: Windows reports results according to the quota allocated to the user.<br class="">+///<br class="">+/// @param Path Input path.<br class="">+/// @returns a space_info structure filled with the capacity, free, and<br class="">+/// available space on the device \a Path is on. A platform specific error_code<br class="">+/// is returned on error.<br class="">+ErrorOr<space_info> disk_space(const Twine Path);<br class=""></blockquote><div class=""><br class=""></div><div class="">I think you need to change the parameter type to `const Twine &`.<span class="Apple-converted-space"> </span></div></div></div></div></div></blockquote><div class=""><br class=""></div></div></div><div class="">Oh your right, I was about to do it after your comment on phab, but then the windows bot yelled at me and I've been busy fixing the build to the point where I forgot. I'm glad you double-checked!</div><span class=""><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;" class=""><div class="gmail_extra"><div class="gmail_quote"><div class="">As documented in Twine.h, Twine's implementation relies on the ability to store pointers to temporary stock objects which may be deallocated at the end of a statement, so having a Twine as a local variable (or a parameter) is not correct.</div></div></div></div></div></blockquote><div class=""><br class=""></div></span><div class="">You can't take ownership of the Twine, but this is not what happens here, is there a correctness issue (I see the inefficiency issue)? The twine will points to objects that are in the caller frame and won't be deallocated before the end of the call?</div></div></div></blockquote><div class=""><br class=""></div><div class="">I read the comment as there is a correctness issue, but I cannot point out the code which is not safe to use this way, so the comment may be wrong.</div></div></div></div></div></blockquote><div><br class=""></div><div>If this is the comment "A Twine is not intended for use directly and should not be stored, its implementation relies on the ability to store pointers to temporary stack objects which may be deallocated at the end of a statement.", then this is what I'm talking about, this is the same issue as a StringRef (it references other data so you shouldn't store it unless you have guarantee on the lifetime of the referenced data).</div><div><br class=""></div><div>-- </div><div>Mehdi</div><div><br class=""></div></div></body></html>