[PATCH] D140547: Perform access checking to private members in simple requirement.

usaxena95 marked 4 inline comments as done. usaxena95 added a comment. I have deleted the test in https://github.com/llvm/llvm-project/commit/a3b632ab8772237ae23638f702bdceda028b2016. It is safe to delete as this is a brittle test trying to generate an invalid requirement using very deep templat

[PATCH] D140547: Perform access checking to private members in simple requirement.

erichkeane added a subscriber: aaron.ballman. erichkeane added a comment. In D140547#4050936 , @ilya-biryukov wrote: > In D140547#4050752 , @uabelho wrote: > >> Anyone else see this? > > I have not checked, but I

[PATCH] D140547: Perform access checking to private members in simple requirement.

ilya-biryukov added a comment. In D140547#4050752 , @uabelho wrote: > Anyone else see this? I have not checked, but I would not be surprised if we hit the stack size limits with asan enabled @usaxena95, maybe reduce the number of instantiations from `10

[PATCH] D140547: Perform access checking to private members in simple requirement.

uabelho added a comment. Hi, When I run the new testcase clang/test/SemaCXX/invalid-requirement-requires-expr.cpp with ASAN binaries the test fails like FAIL: Clang :: SemaCXX/invalid-requirement-requires-expr.cpp (1 of 1) TEST 'Clang :: SemaCXX/invalid-requirement-req

[PATCH] D140547: Perform access checking to private members in simple requirement.

RKSimon added inline comments. Comment at: clang/docs/ReleaseNotes.rst:739 +- Correctly handle access-checks in requires expression. Fixes `GH53364 `_, + `GH53334 `_. C++2b Fe

[PATCH] D140547: Perform access checking to private members in simple requirement.

erichkeane added inline comments. Comment at: clang/docs/ReleaseNotes.rst:739 +- Correctly handle access-checks in requires expression. Fixes `GH53364 `_, + `GH53334 `_. C++2b

[PATCH] D140547: Perform access checking to private members in simple requirement.

RKSimon added inline comments. Comment at: clang/docs/ReleaseNotes.rst:739 +- Correctly handle access-checks in requires expression. Fixes `GH53364 `_, + `GH53334 `_. C++2b Fe

[PATCH] D140547: Perform access checking to private members in simple requirement.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9e0474fbb9c5: Perform access checking to private members in simple requirement. (authored by usaxena95). Changed prior to commit: https://reviews.

[PATCH] D140547: Perform access checking to private members in simple requirement.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM, thanks for fixing this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140547/new/ https://reviews.llvm.org/D140547

[PATCH] D140547: Perform access checking to private members in simple requirement.

usaxena95 updated this revision to Diff 487866. usaxena95 added a comment. Remove new lines. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140547/new/ https://reviews.llvm.org/D140547 Files: clang/include/clang/AST/ExprConcepts.h clang/lib/Par

[PATCH] D140547: Perform access checking to private members in simple requirement.

usaxena95 added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1375 +if (Trap.hasErrorOccurred()) + TransReq.getAs()->setSatisfied(false); + } ilya-biryukov wrote: > ilya-biryukov wrote: > > `TransReq` may be `ExprEr

[PATCH] D140547: Perform access checking to private members in simple requirement.

usaxena95 updated this revision to Diff 487865. usaxena95 marked 6 inline comments as done. usaxena95 added a comment. Addressed comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140547/new/ https://reviews.llvm.org/D140547 Files: clang/in

[PATCH] D140547: Perform access checking to private members in simple requirement.

ilya-biryukov added a comment. I think the only major problem is not checking for error case when accessing `TransReq`, the rest are NITs. Thanks for the change! Will be happy to LGTM it as soon as the access to `TransReq` is fixed. Comment at: clang/lib/Parse/ParseExprCXX.cp

[PATCH] D140547: Perform access checking to private members in simple requirement.

usaxena95 updated this revision to Diff 486604. usaxena95 added a comment. Use ParsingDeclRAIIObject instead of ContextRAII. This creates a separate diagnostic pool for diagnositcs associated to the RequiresExprBodyDecl. This is important because dependent diagnostics should not leak to higher

[PATCH] D140547: Perform access checking to private members in simple requirement.

usaxena95 updated this revision to Diff 486357. usaxena95 added a comment. Return a valid RequriesExpr instead of a ExprError in case of depenedent diagnositcs. We would also need to store the diagnositcs in the RequiresExpr for better diagnosis. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D140547: Perform access checking to private members in simple requirement.

usaxena95 updated this revision to Diff 486313. usaxena95 marked an inline comment as done. usaxena95 added a comment. Added more tests. Still investigating libcxx failure and tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140547/new/ https:/

[PATCH] D140547: Perform access checking to private members in simple requirement.

usaxena95 marked an inline comment as done. usaxena95 added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1367 + if (E->getBody()->isDependentContext()) { +Sema::SFINAETrap Trap(SemaRef); +// We recreate the RequiresExpr body, but no

[PATCH] D140547: Perform access checking to private members in simple requirement.

usaxena95 updated this revision to Diff 486255. usaxena95 marked 3 inline comments as done. usaxena95 added a comment. Adding access check related changes from https://reviews.llvm.org/D140876 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140547/new

[PATCH] D140547: Perform access checking to private members in simple requirement.

ilya-biryukov added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1367 + if (E->getBody()->isDependentContext()) { +Sema::SFINAETrap Trap(SemaRef); +// We recreate the RequiresExpr body, but not by instantiating it.

[PATCH] D140547: Perform access checking to private members in simple requirement.

usaxena95 updated this revision to Diff 485726. usaxena95 added a comment. Improved comment and added comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140547/new/ https://reviews.llvm.org/D140547 Files: clang/lib/Sema/SemaTemplateInstantia

[PATCH] D140547: Perform access checking to private members in simple requirement.

usaxena95 updated this revision to Diff 485725. usaxena95 added a comment. Perform access-dependent checks after transforming requires clause. This is more generic and less invasive than the previous version which changed `RebuildMemberExpr` and also produced duplicate diagnostics. Repository:

[PATCH] D140547: Perform access checking to private members in simple requirement.

shafik added a comment. Thank you for the fix, can you elaborate on the motivation for this change, it looks like setting `setNamingClass` allows for improved diagnostics but I would like it explained better in the PR description. Comment at: clang/lib/Sema/SemaTemplateInstan

[PATCH] D140547: Perform access checking to private members in simple requirement.

usaxena95 created this revision. Herald added a project: All. usaxena95 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D140547 Files: clang/lib/Sema/SemaTemplateInstantiate