Skip to content
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.

Sign up for GitHub

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

[Clang] Fix constant evaluating a captured variable in a lambda #68090

Merged
merged 2 commits into from
Oct 5, 2023
Merged

[Clang] Fix constant evaluating a captured variable in a lambda #68090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d3c5351
[Clang] Fix constant evaluating a captured variable in a lambda
cor3ntin Oct 3, 2023
535fda1
Address Timm's feedback
cor3ntin Oct 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion clang/lib/AST/ExprConstant.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8367,7 +8367,13 @@ bool LValueExprEvaluator::VisitVarDecl(const Expr *E, const VarDecl *VD) {

if (auto *FD = Info.CurrentCall->LambdaCaptureFields.lookup(VD)) {
// Start with 'Result' referring to the complete closure object...
Result = *Info.CurrentCall->This;
if (auto *MD = cast<CXXMethodDecl>(Info.CurrentCall->Callee);
Fznamznon marked this conversation as resolved.
Show resolved Hide resolved
MD->isExplicitObjectMemberFunction()) {
APValue *RefValue =
Info.getParamSlot(Info.CurrentCall->Arguments, MD->getParamDecl(0));
Result.setFrom(Info.Ctx, *RefValue);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what is the difference between calling setFrom and assigning to Result using assignment here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LValue has no assignment operator from APValue (because sometimes you need an ASTContext)

} else
Result = *Info.CurrentCall->This;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the CurrentCall->this be set to the first argument for explicit member functions, somewhere where the stack frame is created?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather not pretend that there is a this object even though it would not correspond to an actual this pointer (and evaluating this is ill formed. I imagine it might make things more difficult to maintain in the future

Comment on lines +8375 to +8376
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think LLVM code style suggests curleys in case other branch had them

Suggested change
} else
Result = *Info.CurrentCall->This;
} else {
Result = *Info.CurrentCall->This;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix, thanks!

// ... then update it to refer to the field of the closure object
// that represents the capture.
if (!HandleLValueMember(Info, E, Result, FD))
Expand Down
18 changes: 18 additions & 0 deletions clang/test/SemaCXX/cxx2b-deducing-this-constexpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,21 @@ consteval void test() {
static_assert(*s == 42);
static_assert((s << 11) == 31);
}

namespace GH68070 {

constexpr auto f = [x = 3]<typename Self>(this Self&& self) {
return x;
};

auto g = [x = 3]<typename Self>(this Self&& self) {
return x;
};

int test() {
constexpr int a = f();
static_assert(a == 3);
return f() + g();
}

}