This is an archive of the discontinued LLVM Phabricator instance.

Disallow ArrayRef assignment from temporaries
ClosedPublic

Authored by jordan_rose on Oct 10 2016, 12:35 PM.

Details

Summary

Without this, the following statements will create ArrayRefs that refer to temporary storage that goes out of scope by the end of the line:

someArrayRef = getSingleElement();
someArrayRef = {elem1, elem2};

Note that the constructor still has this problem:

ArrayRef<Element> someArrayRef = getSingleElement();
ArrayRef<Element> someArrayRef = {elem1, elem2};

but that's a little harder to get rid of because we want to be able to use this in calls:

takesArrayRef(getSingleElement());
takesArrayRef({elem1, elem2});

Part of rdar://problem/16375365.

Diff Detail

Repository
rL LLVM

Event Timeline

jordan_rose retitled this revision from to Disallow ArrayRef assignment from temporaries.
jordan_rose updated this object.
jordan_rose added reviewers: chandlerc, dexonsmith.
jordan_rose set the repository for this revision to rL LLVM.
jordan_rose updated this object.
jordan_rose added a subscriber: llvm-commits.

FWIW, LLVM, Clang, and clang-tools-extra had no issues in this area. Swift had a few. (Unfortunately Swift also has had a few with the constructor involved, so it would be great if we could figure out how to make that safer as well.)

This comment was removed by jordan_rose.
jordan_rose added a comment.EditedOct 10 2016, 1:48 PM

LGTM (with or without the name) if you add std::is_assignable-based static_asserts to unittests/ADT/ArrayRefTest.cpp.

Ooh, I was wondering how to test it. That's a good one.

I'm a little sad but ultimately in favor of making those constructors explicit (or removed altogether with an error message pointing to makeArrayRef), but that's an incredibly breaking change for all clients of LLVM. This one at least should only catch true issues.

jordan_rose accepted this revision.Oct 10 2016, 2:06 PM
jordan_rose added a reviewer: jordan_rose.

Approved by Duncan via email. Committed as r283798.

This revision is now accepted and ready to land.Oct 10 2016, 2:06 PM
jordan_rose closed this revision.Oct 10 2016, 2:07 PM