- 
                Notifications
    You must be signed in to change notification settings 
- Fork 15k
[Clang][Sema] Make lambda in non-dependent context generate same analysis-based warnings as function[ template] #159364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -1972,6 +1972,10 @@ ExprResult Sema::ActOnLambdaExpr(SourceLocation StartLoc, Stmt *Body) { | |
| if (LSI.CallOperator->hasAttr<SYCLKernelEntryPointAttr>()) | ||
| SYCL().CheckSYCLEntryPointFunctionDecl(LSI.CallOperator); | ||
|  | ||
| // TODO: Find out if passing LSI.CallOperator->getDescribedFunctionTemplate() | ||
| // is better when it is a generic lambda. Are there any behaviour | ||
| // changes? `FunctionTemplateDecl` is always passed when handling simple | ||
| // function templates. | ||
| ActOnFinishFunctionBody(LSI.CallOperator, Body, /*IsInstantiation=*/false, | ||
| /*RetainFunctionScopeInfo=*/true); | ||
|  | ||
|  | @@ -2149,24 +2153,28 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, | |
| CleanupInfo LambdaCleanup = LSI->Cleanup; | ||
| bool ContainsUnexpandedParameterPack = LSI->ContainsUnexpandedParameterPack; | ||
| bool IsGenericLambda = Class->isGenericLambda(); | ||
| sema::AnalysisBasedWarnings::Policy WP = | ||
| AnalysisWarnings.getPolicyInEffectAt(EndLoc); | ||
|  | ||
| CallOperator->setLexicalDeclContext(Class); | ||
| Decl *TemplateOrNonTemplateCallOperatorDecl = | ||
| CallOperator->getDescribedFunctionTemplate() | ||
| ? CallOperator->getDescribedFunctionTemplate() | ||
| : cast<Decl>(CallOperator); | ||
| 
      Comment on lines
    
      -2154
     to 
      -2157
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can leave it as-is? That way you can sema::AnalysisBasedWarnings::Policy *ActivePolicy = IsGenericLambda ? nullptr : &WP;
Sema::PoppedFunctionScopePtr _ =
      PopFunctionScopeInfo(ActivePolicy, TemplateOrNonTemplateCallOperatorDecl);Though I have no idea why  | ||
| Decl *TemplateOrNonTemplateCallOperatorDecl = CallOperator; | ||
| sema::AnalysisBasedWarnings::Policy *ActivePolicy = &WP; | ||
| if (IsGenericLambda) { | ||
| TemplateOrNonTemplateCallOperatorDecl = | ||
| CallOperator->getDescribedFunctionTemplate(); | ||
| ActivePolicy = nullptr; | ||
| } | ||
|  | ||
| // FIXME: Is this really the best choice? Keeping the lexical decl context | ||
| // set as CurContext seems more faithful to the source. | ||
| TemplateOrNonTemplateCallOperatorDecl->setLexicalDeclContext(Class); | ||
|  | ||
| PopExpressionEvaluationContext(); | ||
|  | ||
| sema::AnalysisBasedWarnings::Policy WP = | ||
| AnalysisWarnings.getPolicyInEffectAt(EndLoc); | ||
| // We cannot release LSI until we finish computing captures, which | ||
| // requires the scope to be popped. | ||
| Sema::PoppedFunctionScopePtr _ = PopFunctionScopeInfo(&WP, LSI->CallOperator); | ||
| Sema::PoppedFunctionScopePtr _ = | ||
| PopFunctionScopeInfo(ActivePolicy, TemplateOrNonTemplateCallOperatorDecl); | ||
|  | ||
| // True if the current capture has a used capture or default before it. | ||
| bool CurHasPreviousCapture = CaptureDefault != LCD_None; | ||
|  | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -178,3 +178,41 @@ auto b() { | |
| } | ||
| } // namespace test6 | ||
| #endif | ||
|  | ||
| #if __cplusplus >= 201402L | ||
| // ensure lambda in non-dependent context generate same diagnostics as function[ template] | ||
| namespace lambda_in_non_dependent_context { | ||
| void f1() { | ||
| 0, 0; // expected-warning {{left operand of comma operator has no effect}} | ||
| return; | ||
| 0, 0; | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why don't we have warnings for non-generic lambdas? Is that pre-existing issue? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have warnings controlled by  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But with your patch, we do have after-return warnings surfaced for generic-lambdas. This makes us inconsistent. Edit: I see we are already inconsistent with non-templates: https://gcc.godbolt.org/z/PsqYTrMEv Does -Wunreachable-code change anything? The warnings are still not very great to me. | ||
| } | ||
| template <typename T> void f2(T) { | ||
| 0, 0; // expected-warning {{left operand of comma operator has no effect}} | ||
| return; | ||
| 0, 0; // expected-warning {{left operand of comma operator has no effect}} | ||
| } | ||
| auto L1 = [] { | ||
| 0, 0; // expected-warning {{left operand of comma operator has no effect}} | ||
| return; | ||
| 0, 0; | ||
| }; | ||
| auto L2 = [](auto) { | ||
| 0, 0; // expected-warning {{left operand of comma operator has no effect}} | ||
| return; | ||
| 0, 0; // expected-warning {{left operand of comma operator has no effect}} | ||
| }; | ||
| void f() { | ||
| auto L1 = [] { | ||
| 0, 0; // expected-warning {{left operand of comma operator has no effect}} | ||
| return; | ||
| 0, 0; | ||
| }; | ||
| auto L2 = [](auto) { | ||
| 0, 0; // expected-warning {{left operand of comma operator has no effect}} | ||
| return; | ||
| 0, 0; // expected-warning {{left operand of comma operator has no effect}} | ||
| }; | ||
| } | ||
| } | ||
| #endif | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is still here; I'd like to suggest we remove it because it doesn't look very clear: what behavior changes are we talking about/which function that Decl is passed to?