[Sema] LambdaScopeForCallOperatorInstantiationRAII - fix typo in early out logic

[RKSimon]

We should be checking for a failed dyn_cast on the ParentFD result - not the loop invariant FD root value.

Seems to have been introduced in #65193

Noticed by static analyser (I have no specific test case).

[llvmbot]

@llvm/pr-subscribers-clang

Author: Simon Pilgrim (RKSimon)

Changes

We should be checking for a failed dyn_cast on the ParentFD result - not the loop invariant FD root value.

Seems to have been introduced in #65193

Noticed by static analyser (I have no specific test case).

---

Full diff: https://github.com/llvm/llvm-project/pull/96888.diff

1 Files Affected:

• (modified) clang/lib/Sema/SemaLambda.cpp (+1-1)

diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index e9476a0c93c5d..ca9c7cb9faadf 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -2391,7 +2391,7 @@ Sema::LambdaScopeForCallOperatorInstantiationRAII::
       Pattern =
           dyn_cast<FunctionDecl>(getLambdaAwareParentOfDeclContext(Pattern));
 
-      if (!FD || !Pattern)
+      if (!ParentFD || !Pattern)
         break;
 
       SemaRef.addInstantiatedParametersToScope(ParentFD, Pattern, Scope, MLTAL);

[zyn0217]

Thanks for the patch.
While I think this makes sense, it still confuses me whether there was some redundancy in the guard condition. Do we have any situation where Pattern is non-null whereas ParentFD becomes null? I assume they are always paired, at least for the lambda cases.

[RKSimon]

It would probably explain why the typo hasn't caused regression - I'm going off static analysis reports here, not crash reports.

[zyn0217]

Looks like we have hit the assert in SemaTemplate/alias-template-with-lambdas.cpp. I brought that dirty test, and I'll take a look this weekend.

[0x59616e]

This is indeed a typo.

[0x59616e]

Thanks for this fix!

[RKSimon]

Cheers - I'll push this (without the assert) in a moment - that shouldn't get in the way of working out why they aren't always paired.

[zyn0217]

So, here is the offending case:

template <class>
concept True = true;

template <class... U>
using MeowMeow = decltype([]<True>(U...) {}.template operator()<char>(U()...));

void foo() {
  using T = MeowMeow<char, int, long, unsigned>;
}

In this example, the lambda does not have the TypeAliasTemplateDecl where it is defined as its DeclContext - the TypeAliasTemplateDecl / TypeAliasDecl does not constitute a DeclContext. So, it's owned by TranslationUnitDecl. OTOH, we will instantiate the lambda inside the function foo() and thus the DeclContext of the instantiation is foo(), hence the discrepancy.

[zyn0217]

Thinking more, I still maintain that the check for ParentFD is redundant.

So, suppose we need to find a case to compromise the previous logic. In that case, we need to find a generic lambda whose Pattern is defined inside a function (i.e. Pattern is non-null) while its Instantiation (i.e. ParentFD is null) is not. I don't think that is possible because we can't instantiate any lambda definition outside the function that owns them.

That being said, I think keeping the patch as-is is fine because it's more intelligible than explaining the subtle implications above.