[Clang] Fix evaluation of parameters of lambda call operator attributes

[cor3ntin]

Summary

Fix a regresion introduced by D124351.
Attributes of lambda call operator were evaluated in the
context of the closure object type rather than its operator,
causing an assertion failure.

This was because we temporarily switch to the class lambda to
produce the mangling of the lambda, but we stayed in that
context too long.

[cor3ntin]

Whether the assertion itself in getCurLambda is still pertinent,
despite no test depending on it does require more analysis.

This comment in SemaExprCXX.cpp dissuaded to try to do more surgery as part of this fix, and in some way, that assertion firing helped us discover the bug this PR fixes, so it was useful

// FIXME: PR 17877 showed that getCurLambda() can return a valid pointer
// even if CurContext is not a lambda call operator. Refer to that Bug Report
// for an example of the code that might cause this asynchrony.
// By ensuring we are in the context of a lambda's call operator
// we can fix the bug (we only need to check whether we need to capture
// if we are within a lambda's body); but per the comments in that
// PR, a proper fix would entail :
//   "Alternative suggestion:
//   - Add to Sema an integer holding the smallest (outermost) scope
//     index that we are *lexically* within, and save/restore/set to
//     FunctionScopes.size() in InstantiatingTemplate's
//     constructor/destructor.
//  - Teach the handful of places that iterate over FunctionScopes to
//    stop at the outermost enclosing lexical scope."

[eandrews]

Attributes of lambda call operator were evaluated in the context of the closure object type rather than its operator,

Just for my understanding, what did this affect with regards to the if guard for assert? CurContext or something else? I suspected it had something to do with contexts being incorrect but I wasn't familiar enough with the code to pinpoint where/what exactly.

[cor3ntin]

In D146535#4210198, @eandrews wrote:

Attributes of lambda call operator were evaluated in the context of the closure object type rather than its operator,

Just for my understanding, what did this affect with regards to the if guard for assert? CurContext or something else? I suspected it had something to do with contexts being incorrect but I wasn't familiar enough with the code to pinpoint where/what exactly.

Exactly CurContext. For the purpose of mangling lambda the context needs to be the parent of the lambda class, so !CurLSI->Lambda->Encloses(CurContext) - which check that we are actually in the lambda - was (incorrectly) true.

Note that you are right about the assert being fishy to begin with but that's a preexisting thing and i can't quite convince myself than removing it entirely wouldn't break something. That we were in the wrong context is something i broke :(

[eandrews]

In D146535#4210268, @cor3ntin wrote:

In D146535#4210198, @eandrews wrote:

Attributes of lambda call operator were evaluated in the context of the closure object type rather than its operator,

Just for my understanding, what did this affect with regards to the if guard for assert? CurContext or something else? I suspected it had something to do with contexts being incorrect but I wasn't familiar enough with the code to pinpoint where/what exactly.

Exactly CurContext. For the purpose of mangling lambda the context needs to be the parent of the lambda class, so !CurLSI->Lambda->Encloses(CurContext) - which check that we are actually in the lambda - was (incorrectly) true.

Note that you are right about the assert being fishy to begin with but that's a preexisting thing and i can't quite convince myself than removing it entirely wouldn't break something. That we were in the wrong context is something i broke :(

Thanks for explaining!

[aaron.ballman]

LGTM

[eandrews]

Can we merge this patch? It will fix regressions introduced by D124351 I see downstream

[cor3ntin]

Sure! I didn't land it sooner because the bots were down. but apparently they were up quite fast. Thanks!

clang/lib/Sema/SemaLambda.cpp

if (Class->isDependentContext() || TemplateParams) {
     }
   }
   return MethodType;
 }
 
 void Sema::handleLambdaNumbering(
     CXXRecordDecl *Class, CXXMethodDecl *Method,
     std::optional<std::tuple<bool, unsigned, unsigned, Decl *>> Mangling) {
+
+  ContextRAII ManglingContext(*this, Class->getDeclContext());
+
   if (Mangling) {
     bool HasKnownInternalLinkage;
     unsigned ManglingNumber, DeviceManglingNumber;
     Decl *ManglingContextDecl;
     std::tie(HasKnownInternalLinkage, ManglingNumber, DeviceManglingNumber,
              ManglingContextDecl) = *Mangling;
     Class->setLambdaMangling(ManglingNumber, ManglingContextDecl,
                              HasKnownInternalLinkage);
bool IsLambdaStatic =
       ParamInfo.getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_static;
 
   CompleteLambdaCallOperator(
       Method, Intro.Range.getBegin(), CallOperatorLoc,
       ParamInfo.getTrailingRequiresClause(), MethodTyInfo,
       ParamInfo.getDeclSpec().getConstexprSpecifier(),
       IsLambdaStatic ? SC_Static : SC_None, Params, ExplicitResultType);
 
-  ContextRAII ManglingContext(*this, Class->getDeclContext());
-
   CheckCXXDefaultArguments(Method);
 
   // This represents the function body for the lambda function, check if we
   // have to apply optnone due to a pragma.
   AddRangeBasedOptnone(Method);
 
   // code_seg attribute on lambda apply to the method.
   if (Attr *A = getImplicitCodeSegOrSectionAttrForFunction(
           Method, /*IsDefinition=*/true))
     Method->addAttr(A);
 
   // Attributes on the lambda apply to the method.
   ProcessDeclAttributes(CurScope, Method, ParamInfo);
 
   // CUDA lambdas get implicit host and device attributes.
   if (getLangOpts().CUDA)
     CUDASetLambdaAttrs(Method);
 
   // OpenMP lambdas might get assumumption attributes.
   if (LangOpts.OpenMP)
     ActOnFinishedFunctionDefinitionInOpenMPAssumeScope(Method);
 
   handleLambdaNumbering(Class, Method);
 
-  ManglingContext.pop();
-
   for (auto &&C : LSI->Captures) {
     if (!C.isVariableCapture())
       continue;
     ValueDecl *Var = C.getVariable();
     if (Var && Var->isInitCapture()) {
       PushOnScopeChains(Var, CurScope, false);
     }
   }

clang/test/SemaCXX/lambda-expressions.cpp

 // RUN: %clang_cc1 -std=c++14 -Wno-unused-value -fsyntax-only -verify -verify=expected-cxx14 -fblocks %s
-// RUN: %clang_cc1 -std=c++17 -Wno-unused-value -fsyntax-only -verify -fblocks %s
+// RUN: %clang_cc1 -std=c++17 -Wno-unused-value -verify -ast-dump -fblocks %s | FileCheck %s
 
 namespace std { class type_info; };
 
 namespace ExplicitCapture {
   class C {
     int Member;
 
     static void Overload(int);

 };
 
 static_assert([]() constexpr {
   S r = S{2};
   return r.p != nullptr;
 }());
 } // namespace GH60936
 #endif
+
+// Call operator attributes refering to a variable should
+// be properly handled after D124351
+constexpr int i = 2;
+void foo() {
+  (void)[=][[gnu::aligned(i)]] () {}; // expected-warning{{C++2b extension}}
+  // CHECK: AlignedAttr
+  // CHECK-NEXT: ConstantExpr
+  // CHECK-NEXT: value: Int 2
+}