Fix std::ref(i) with std::reference_wrapper

[mgehre]

See

std::reference_wrapper<int> danglingPtrFromNonOwnerLocal2() {
  int i = 5;
  return std::ref(i); // TODOexpected-warning {{address of stack memory associated with local variable 'i' returned}}
}

in test/Sema/warn-lifetime-analysis-nocfg.cpp.
fyi @Xazax-hun

[Xazax-hun]

I will look into that as soon as we are done with Richard's request :) I was actually investigating this when I noticed the other problem.

[Xazax-hun]

I'm not sure if we can fix this without having the DerefType. We figured out that we do not want to warn if we create a Pointer from an unannotated type.

[mgehre]

We have

template< class T >
std::reference_wrapper<T> ref(T& t);

Here the parameter t is a reference, so it's a Pointer. We can annotate the function
with [[gsl::post(gsl::lifetime(return, {t}))]] because the pset of the return value is the same as the pset of t. Both point to the thing of type T that the reference points to.
For this T, does not need to be an Owner or Pointer. It could e.g. be int.

The same applies to the std::reference_wrapper constructor. We figured out that we don't want to automatically infer pset(*this) = pset(t) when t is a reference to an unannotated type, but we can have an explicit annotation on that particular constructor.

It's true that when we have a DerefType, the automatic function annotations inference would come to the same conclusion.

Separately, we might need to annotate that t is not an output parameter, even though its passed by non-const ref.

[Xazax-hun]

Yeah, I see that. But in this sense std::ref is different from std::begin. Maybe we can add separate codepath for std::ref, but not sure of it worth it, since it might be unlikely that the users write such code anyways. This problem will be solved once we have annotations. The question is, how to annotate std::begin and the likes? Implicit annotations will probably be only added for certain instantiations. But this would mean that the user also needs a way to only annotate some instantiations.

[Xazax-hun]

To give a bit more context, the reason why is it not easy to add a separate code path currently:
The current code builds a vector called Path. This Path records what happened so far, e.g. we constructed a GslPointer. But it does not record which pointer was constructed (e.g. if it was a reference_wrapper). So it is not easy to handle different pointer types separately. This abstraction is used by other existing warnings and I do not see a way to change this without rewriting the code for existing warnings.

[mgehre]

Ok, I didn't know that. That seems to imply that we cannot support std::ref (right now), correct?

I tried to formulate explicit annotations on both functions, and now I'm a bit confused about our annotations syntax. What I initially came up with is

template<typename T>
reference_wrapper<T> ref(T& t) [[gsl::post(lifetime(return, {t}))]]

template<typename T>
auto begin(T& t) [[gsl::post(lifetime(return, {t}))]]

i.e. the same written contract in both cases, but meaning different things:

• for std::ref, the pset of the return value is equal to the pset of the reference
• for std::begin, the pset of the return value is equal to the pset of what the reference refers to
  To clarify, should I have written
  auto begin(T& t) [[gsl::post(lifetime(return, {deref(t)}))]] to look through the toplevel reference?

[Xazax-hun]

Yeah, unfortunately. There might be a way to make it work though, but it needs more thought.

For your question, as far as I remember, the answer is yes. Last time we agreed on the reference meaning the pset of the top level thing and we need to use deref to get the pset of the pointee. An alternative would be to use & in the first case but Herb did not like that idea.