Missing warning when binding to const ref?

I’m surprised this doesn’t produce a warning:

struct S
{
S(const int& i) : i(i) {}
const int& i;
};

void foo(long l)
{
S s(l); // binding a const ref to temporary
}

int main()
{
foo(1l);
}

l-value reference. And, function foo takes the parameter by value, so it
is passing an l-value to the constructor of S.

I should have made a better comment, the types are different! Parameter needs to be implicitly converted to int, and we’re binding a a const ref to this temp object whose lifetime ends once S is constructed. Unless I’m mistaken.

Ah! I finally see where your issue is. This problem is probably better suited for static code analysis. For a trivial example like this you can pretty quickly track down what is wrong, but for the more general case I don’t imagine that this being a check that can be done quickly enough for a compiler warning.

We just ran into this in production, but as you can imagine it was much harder to spot. It only happened in optimized code. It was hidden behind templates and one member of the class would try to bind a const ref to int to something that was an enum. Combine that with mfc hash map and results are spectacular :slight_smile:

Looking at ast dump I can see MaterializeTemporaryExpr. Couldn’t we just check that one of function’s or constructor’s arguments is coming from it?

So, it would be something like:
If MaterializeTemporaryExpr and assign result to a reference that persists beyond the scope of the function
Then generate a warning

Something like that? I am not an expert on this; could someone else comment on the reasonableness of this?

Yeah, something like that, it really feels like it belongs in the static analyzer. BTW, would any of the sanitizers catch this?

Yeah, something like that, it really feels like it belongs in the static
analyzer. BTW, would any of the sanitizers catch this?

I believe MSan would be the tool to catch this, but I'm not sure how good
its stack lifetime tracking is just yet.

(& yes, I don't think there's a local warning (non-static analysis) that
would be good for this. Stashing a reference to a temporary in a member
variable isn't, itself, a bug - see Twine for example - you just have to
use it carefully. That said, we did have some bugs in LLVM/Clang due to
taking references to temporaries from conversions - ArrayRef<Base*>
constructed from a single Derived* - the Derived* was converted to a
temporary Base*, the ArrayRef had a pointer to that Base* which ceased to
exist at the end of the full expression... )