[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

aaron.ballman added a comment. In D140828#4653389 , @nikic wrote: > FYI this causes some compile-time regression: > http://llvm-compile-time-tracker.com/compare.php?from=bc7d88faf1a595ab59952a2054418cdd0d98&to=af4751738db89a142a8880c782d12d4201b222a8

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

nikic added a comment. FYI this causes some compile-time regression: http://llvm-compile-time-tracker.com/compare.php?from=bc7d88faf1a595ab59952a2054418cdd0d98&to=af4751738db89a142a8880c782d12d4201b222a8&stat=instructions:u About 0.7% for C++ code at `O0`. Sample for a file with >2% would be

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

zequanwu added a comment. Hi, compiling llvm-10 code with this change causing compilation errors: ../../third_party/swiftshader/third_party/llvm-10.0/llvm/include/llvm/ADT/Optional.h:289:8: error: cannot overload a member function with ref-qualifier '&&' with a member function without a ref-

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

Endill added inline comments. Comment at: clang/docs/ReleaseNotes.rst:94-98 +- Implemented `P0847R7: Deducing this `. Some related core issues were also + implemented (`CWG2553 `, `CWG2554 `, + `

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Accepting as-is; we'll keep our eyes peeled for any post-commit feedback. Thank you for all the hard work on this one, it was a big, involved patch! Repository: rG LLVM Github

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

aaron.ballman added a comment. Aside from some minor nits in the release notes, I think this LGTM. I'm going to hold off on accepting officially for a day or two just so other code reviews have a chance to weigh in (the codegen code owners were only added to the review today). ==

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin marked 7 inline comments as done. cor3ntin added inline comments. Comment at: clang/include/clang/Sema/Sema.h:3775 bool UseMemberUsingDeclRules, bool ConsiderCudaAttrs = true, - bool ConsiderRequiresClauses = true); +

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

aaron.ballman added inline comments. Comment at: clang/test/SemaCXX/cxx2b-deducing-this.cpp:192 +f(); // expected-error{{call to non-static member function without an object argument}} +f(Over_Call_Func_Example{}); // expected-error{{call to non-static m

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

aaron.ballman added inline comments. Comment at: clang/include/clang/Sema/Sema.h:3775 bool UseMemberUsingDeclRules, bool ConsiderCudaAttrs = true, - bool ConsiderRequiresClauses = true); + bool UseOverrideRules = false);

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin added inline comments. Comment at: clang/include/clang/AST/ASTLambda.h:45 + return isLambdaCallOperator(DC) && + !cast(DC)->getType().isNull() && + !cast(DC)->isExplicitObjectMemberFunction(); aaron.ballman wrote: > cor3ntin wrote: > > e

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

aaron.ballman added reviewers: rjmccall, efriedma. aaron.ballman added a comment. I think we're getting pretty close! My goal is to get this landed ASAP; I do not think it needs to be hidden behind a feature flag (-std=c++2b is sufficient), and it's good that we're not defining the feature test

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin added inline comments. Comment at: clang/test/CodeGenCXX/cxx2b-deducing-this.cpp:4 + +struct TrivialStruct { +void explicit_object_function(this TrivialStruct) {} cor3ntin wrote: > aaron.ballman wrote: > > I'd like to see more codegen tests in genera

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin added inline comments. Comment at: clang/lib/AST/DeclCXX.cpp:841 const auto *ParamTy = - Method->getParamDecl(0)->getType()->getAs(); + Method->getNonObjectParameter(0)->getType()->getAs(); if (!ParamTy || ParamTy->getPointeeType().isConst

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin marked 3 inline comments as done. cor3ntin added inline comments. Comment at: clang/include/clang/AST/ASTLambda.h:45 + return isLambdaCallOperator(DC) && + !cast(DC)->getType().isNull() && + !cast(DC)->isExplicitObjectMemberFunction(); e

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

erichkeane added a comment. Mostly just nits (plus my hatred for C style casts :) ), I didn't see any issues with the approach. Comment at: clang/include/clang/AST/ASTLambda.h:45 + return isLambdaCallOperator(DC) && + !cast(DC)->getType().isNull() && + !cast(D

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

steakhal added a comment. I checked out the code to see how does the Static Analyzer work after this. I'm impressed that it seems to work. Do you mind adding my test file to this patch? `clang/test/Analysis/cxx2b-deducing-this.cpp`: // RUN: %clang_analyze_cc1 -std=c++2b -verify %s \ // RUN:

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:13033 + ExprResult Res = FixOverloadedFunctionReference(E, DAP, Found); + if (Res.isInvalid()) +return false; aaron.ballman wrote: > Below you assume that `Res.get()` returns a nonn

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin marked 2 inline comments as done. cor3ntin added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7260-7265 +def ext_deducing_this : ExtWarn< + "explicit object parameters are a C++2b extension">, + InGroup; +def warn_cxx20_ext_deducing_thi

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin marked 4 inline comments as done. cor3ntin added inline comments. Comment at: clang/test/SemaCXX/cxx2b-deducing-this.cpp:19 + +void g(this auto) const; // expected-error{{a function with an explicit object parameter cannot be const}} +void h(this auto) &; // exp

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin added inline comments. Comment at: clang/test/CXX/drs/dr25xx.cpp:104-106 +struct D : B { + void f(this D&); // expected-error {{an explicit object parameter cannot appear in a virtual function}} +}; aaron.ballman wrote: > I'd like to see two other test

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

aaron.ballman added a reviewer: erichkeane. aaron.ballman added a comment. Phew, that completes my first pass through the review! I'm also adding @erichkeane as a reviewer now that he's off sabbatical. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9378-9381 + "

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:911-915 if (X->getNumParams() != Y->getNumParams()) return false; for (unsigned I = 0; I < X->getNumParams(); ++I) if (!Ctx.hasSameUnqualifiedType(X->getParamDecl(I)->getType(),

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin added inline comments. Comment at: clang/include/clang/Sema/Sema.h:3775 bool UseMemberUsingDeclRules, bool ConsiderCudaAttrs = true, - bool ConsiderRequiresClauses = true); + bool UseOverrideRules = false); -

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

aaron.ballman added inline comments. Comment at: clang/include/clang/Sema/Sema.h:3775 bool UseMemberUsingDeclRules, bool ConsiderCudaAttrs = true, - bool ConsiderRequiresClauses = true); + bool UseOverrideRules = false);

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin marked 2 inline comments as done. cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:4330-4331 +ExprResult Res = FixOverloadedFunctionReference(From, Found, Fn); +if (Res.isInvalid()) + return ExprError(); cor3ntin

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

tbaeder added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:2063-2064 bool isInstance() const { return !isStatic(); } + bool isExplicitObjectMemberFunction() const; + bool isImplicitObjectMemberFunction() const; + const ParmVarDecl *getNonObjectParameter(un

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:18171 +bool Sema::CheckOverridingExplicitObjectMembers(CXXMethodDecl *New, +const CXXMethodDecl *Old) { cor3ntin wrote: > aaron.ballm

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:18171 +bool Sema::CheckOverridingExplicitObjectMembers(CXXMethodDecl *New, +const CXXMethodDecl *Old) { aaron.ballman wrote: > This functi

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin marked 9 inline comments as done. cor3ntin added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7280 + "a %select{function|lambda}0 with an explicit object parameter cannot " + "%select{be const|be mutable|have reference qualifiers|be vol

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7280 + "a %select{function|lambda}0 with an explicit object parameter cannot " + "%select{be const|be mutable|have reference qualifiers|be volatile}1">; +def err_invalid_explicit_o

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7280 + "a %select{function|lambda}0 with an explicit object parameter cannot " + "%select{be const|be mutable|have reference qualifiers|be volatile}1">; +def err_invalid_explicit_object

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7280 + "a %select{function|lambda}0 with an explicit object parameter cannot " + "%select{be const|be mutable|have reference qualifiers|be volatile}1">; +def err_invalid_explicit_o

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin marked an inline comment as done. cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:11308 + + // There should be a CWG issue for that + if (Name.getNameKind() == DeclarationName::CXXConstructorName || aaron.ballman wrote: > Do

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin marked an inline comment as done. cor3ntin added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:4283 + if (HasExplicitObjectParameter) { +const VarDecl *D = cast(CurCodeDecl)->getParamDecl(0); +auto It = LocalDeclMap.find(D); aaron.ba

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

aaron.ballman added a subscriber: efriedma. aaron.ballman added a comment. Round two of comments; I picked up from where I left off earlier and haven't checked your changes or responses yet. Precommit CI seems to have found a relevant failure from the patch. Comment at: clang

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin added inline comments. Comment at: clang/lib/AST/DeclCXX.cpp:841 const auto *ParamTy = - Method->getParamDecl(0)->getType()->getAs(); + Method->getNonObjectParameter(0)->getType()->getAs(); if (!ParamTy || ParamTy->getPointeeType().isConst

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7280 + "a %select{function|lambda}0 with an explicit object parameter cannot " + "%select{be const|be mutable|have reference qualifiers|be volatile}1">; +def err_invalid_explicit_object

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin added inline comments. Comment at: clang/include/clang/AST/Expr.h:1279 + explicit DeclRefExpr(EmptyShell Empty) : Expr(DeclRefExprClass, Empty) { +DeclRefExprBits.CapturedByCopyInLambdaWithExplicitObjectParameter = false; + } aaron.ballman wrote: >

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

aaron.ballman added a subscriber: rjmccall. aaron.ballman added a comment. I started my review for this, but haven't completed it yet. I figured some early feedback would be better than waiting for me to complete the whole review. Comment at: clang/include/clang/AST/Decl.h:101

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin added a comment. Interesting case that crashes the compiler, stack overflow cpp struct T{}; struct C { operator T (this int); operator int(); }; void foo(C c) { T d = c; } Not completely sure that this is supposed to be covered by https://eel.is/c++draft

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin added a comment. I added a list of core issues I'm aware of in https://github.com/llvm/llvm-project/issues/59619 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140828/new/ https://reviews.llvm.org/D140828 _

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

Zingam added a comment. In D140828#4544438 , @cor3ntin wrote: > In D140828#4544433 , @Zingam wrote: > >> Is this going to land soon? There is a libc++ paper I'm interested in that >> relies on "deducing this" and

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

cor3ntin added a comment. In D140828#4544433 , @Zingam wrote: > Is this going to land soon? There is a libc++ paper I'm interested in that > relies on "deducing this" and I wonder if I should wait for "this" first > before I start working on it? Hard t

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

Zingam added a comment. Is this going to land soon? There is a libc++ paper I'm interested in that relies on "deducing this" and I wonder if I should wait for "this" first before I start working on it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

Endill added a comment. Thank you for working on this feature! I can't review this properly, so here's my little help for the feature I'm looking forward to very much. Comment at: clang/test/CXX/drs/dr25xx.cpp:1 -// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown %s