[PATCH] D44480: [Sema] Don't skip function bodies with 'auto' without trailing return type

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Sorry for delays with this patch, I somehow missed the email notification about it.

> Expounding on my concerns a bit -- I'm worried about all the other places calling isUndeducedType() and whether they're also at risk and thus a better fix is to change isUndeducedType() to pay attention to the language mode.

I thought semantics of `isUndeducedType()` is actually what's most callers (e.g. diagnostics) want. That is "the type is undeduced **and** not dependent". But the name can definitely bring some confusion, so there might be other places that misinterpreted it.

@rsmith, it would be great if you could take another look when you have time.



================
Comment at: lib/Sema/SemaDecl.cpp:12609-12610
       return false;
+    // We can't simply call Type::isUndeducedType here, because it returns
+    // false on C++14's auto return type without trailing return type.
+    DeducedType *DT = FD->getReturnType()->getContainedDeducedType();
----------------
rsmith wrote:
> I don't think this comment captures the problem. Rather, I think the problem is that in a template we deduce the `auto` to a dependent type, so it's not considered "undeduced" at the point when we ask this question.
Thanks! That definitely explains the problem in a proper manner.


================
Comment at: lib/Sema/SemaDecl.cpp:12612-12613
+    DeducedType *DT = FD->getReturnType()->getContainedDeducedType();
+    if (DT && DT->getDeducedType().isNull())
+      return false;
+  }
----------------
rsmith wrote:
> Is there any need to check whether the type has been deduced here? It seems simpler (and equivalent) to turn off skipping for functions whose return type involves a deduced type regardless of whether deduction has been performed already (which it hasn't).
Thanks! I misinterpreted the deduced type's semantics. I thought it also appears in the following case:
`auto foo() -> int {}`
But it doesn't.


Repository:
  rC Clang

https://reviews.llvm.org/D44480