[PATCH] D74094: Reapply: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D74094#4643495 <https://reviews.llvm.org/D74094#4643495>, @rjmccall wrote:

> I can't easily tell you because the "standalone example" involves a million templates that I don't know anything about, and nobody seems to have done the analysis to identify the specific source of the miscompile.

Sure; @alexfh @vitalybuka can you both please help produce a further reduced test case from https://reviews.llvm.org/D74094#4633799 ? Otherwise I'm beginning to think this patch may have been reverted prematurely; before an understood + reduced test case was reduced.

> What I can tell you is that there is an enormous semantic difference between passing a pr-value argument to a parameter that's passed by reference and to a parameter that's passed by value.  In the former case, the reference is bound to a temporary with full-expression lifetime, and it is totally reasonable to return its address.  In the latter case, the parameter object will often have a lifetime bound by the current function, and you cannot return its address without it being UB.

I'm trying to come up with an example of "the former."

  struct foo {
    int x;
  };
  
  // Returns a reference to the minimum foo.
  foo &min(foo &a, foo &b);
  
  void consume (foo&);
  
  void bar () {
    consume(min(foo{0}, foo{1}));
  }

doesn't compile:

  foo.cpp:11:11: error: no matching function for call to 'min'
     11 |   consume(min(foo{0}, foo{1}));
        |           ^~~
  foo.cpp:6:6: note: candidate function not viable: expects an lvalue for 1st argument
      6 | foo &min(foo &a, foo &b);
        |      ^   ~~~~~~

Have I misinterpreted what you meant by "passing a pr-value argument to a parameter that's passed by reference?"

> Looking closer, my previous guess that this was accidentally triggering for reference parameters seems to be wrong, because that case is filtered out earlier in the function.  I'm not sure what the problem is exactly, then, unless the code indeed has UB.

In which case, perhaps this was reverted prematurely.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74094/new/

https://reviews.llvm.org/D74094