This is an archive of the discontinued LLVM Phabricator instance.

[LifetimeAnalysis] Support std::stack::top() and std::optional::value()
ClosedPublic

Authored by mgehre on Aug 13 2019, 2:01 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

mgehre created this revision.Aug 13 2019, 2:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 13 2019, 2:01 PM
xazax.hun accepted this revision.Aug 13 2019, 2:10 PM

LG! But let's wait for Dmitri :)

clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
172 ↗(On Diff #214912)

I actually was a bit lazy when I added these tests. Both value and operator* is overloaded on &&. But if you do not feel like adjusting the tests this is fine, I can do it myself later :)

This revision is now accepted and ready to land.Aug 13 2019, 2:10 PM
xazax.hun added inline comments.Aug 13 2019, 3:04 PM
clang/lib/Sema/SemaInit.cpp
6583 ↗(On Diff #214912)

Oh, this one needs to be updated, as value returns a reference not a pointer.

mgehre marked an inline comment as done.Aug 13 2019, 3:29 PM
mgehre added inline comments.
clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
172 ↗(On Diff #214912)

I'll change it to use the & variant in the test - the && cannot dangle as far as I understand.

xazax.hun added inline comments.Aug 13 2019, 3:32 PM
clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
172 ↗(On Diff #214912)

It can!

Consider the following code:

int &&r = *std::optional(5);
// r dangles here.
mgehre updated this revision to Diff 214950.Aug 13 2019, 3:35 PM
mgehre marked 2 inline comments as done.

Fix commit

mgehre marked 3 inline comments as done.Aug 13 2019, 3:39 PM
mgehre added inline comments.
clang/lib/Sema/SemaInit.cpp
6583 ↗(On Diff #214912)

Yes, I noticed the same.

clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
172 ↗(On Diff #214912)

Oh, sure!

mgehre updated this revision to Diff 214954.Aug 13 2019, 3:40 PM
mgehre marked an inline comment as done.

Add tests for rvalue-ref overloads

gribozavr accepted this revision.Aug 14 2019, 2:16 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 14 2019, 2:55 PM