<div dir="ltr">Yeah, this general issue comes up across the board. Generally it's "don't do that" and we could add some clang-tidy checks to help catch these cases (perhaps even powered by an attribute on the type so it's extensible).<br><br>which is to say: fix the test case, it's buggy, but that's about it</div><br><div class="gmail_quote"><div dir="ltr">On Wed, Aug 24, 2016 at 11:46 AM Keane, Erich via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Sorry for the inline-comment format being weird, I haven't figured out yet how to do '>' stuff in outlook yet :/  Hopefully this is clear enough.<br>
<br>
-----Original Message-----<br>
From: <a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a> [mailto:<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>]<br>
Sent: Wednesday, August 24, 2016 10:55 AM<br>
To: Keane, Erich <<a href="mailto:erich.keane@intel.com" target="_blank">erich.keane@intel.com</a>><br>
Cc: <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
Subject: Re: [llvm-dev] Pointer to temporary issue in ArrayRefTest.InitializerList<br>
<br>
<br>
> On Aug 24, 2016, at 10:48 AM, Keane, Erich via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
><br>
> Hi all-<br>
> I am mostly doing work in Clang (and am new there), so I apologize if this isn't the proper place to mention this.  I appreciate guidance in advance.<br>
><br>
> I was looking into some of the unit tests, and noticed that the ArrayRefTest.InitializerList, and thus the InitializerList constructor of ArrayRef (under normal use-case) hit undefined behavior.  The test does the following:<br>
>  ArrayRef<int> A = { 0, 1, 2, 3, 4 };<br>
>  for (int i = 0; i < 5; ++i)<br>
>    EXPECT_EQ(i, A[i]);<br>
><br>
> For those unfamiliar, ArrayRef is a T* Data/size_t Length pair-type with a std::initializer_list Ctor that simply copies the initializer_list::begin into Data.<br>
><br>
> The issue is that after the assignment, the initializer-list temporary goes out of scope (since it is a temporary), creating a dangling pointer.  This doesn't seem to be an issue for the most part, however compiling the test with -O2 and -fno-merge-all-constants causes this test to fail.<br>
><br>
> I suspect that this should be fixed in 1 of the following ways.  I'm willing to contribute the patch, but would like some guidance as to which the community thinks is the proper solution.<br>
><br>
> 1- "Delete" r-value ctors for ArrayRef.  I did a quick test just deleting r-value std;:initializer list, and discovered quite a few usages of construct-from-temporary (before the build gave up!) that would need to be fixed as well.<br>
<br>
How would we do with ArrayRef as function argument, built from an R-value? Sounds like a valid use-case to me.<br>
[Keane, Erich] Huh, I hadn't thought of that, but that definitely explains why the GSL version of "span" doesn't delete them.  Also explains why the array_view paper doesn't mention this as well.  I guess it is up to the user to beware of this case.    Perhaps the solution is to just audit our usages and see where we've messed up.<br>
<br>
> 2- Implement the r-value ctors to allocate.  This is likely going to require an additional member to capture the fact that this was allocated and thus needs to be free'd.  I suspect that this violates the purpose of the ArrayRef.<br>
<br>
Right.<br>
<br>
Note that I believe the same issue exists with Twine and StringRef.<br>
[Keane, Erich] Interesting... I'm surprised to see that StringRef isn't implemented in terms of ArrayRef (with inheritance).<br>
<br>
—<br>
Mehdi<br>
<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div>