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

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 added inline comments.


================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1375
+        if (Trap.hasErrorOccurred())
+          TransReq.getAs<RequiresExpr>()->setSatisfied(false);
+      }
----------------
ilya-biryukov wrote:
> ilya-biryukov wrote:
> > `TransReq` may be `ExprError` and this will cause a crash. Worth adding a test too.
> Could you please add `assert(!TransReq || *TransReq != E)`.
> The common optimization in TreeTransform is to avoid rebuilding the AST nodes if nothing changes. There is no optimization like this for `RequireExpr` right now, but it would not be unexpected if this gets implemented in the future.
> 
> In those situations, the current code can potentially change value of `isSatisfied` for an existing expression rather than for a newly created, which seems like asking for trouble. It would be nice to give an early warning to implementors of this optimization that they should think how to handle this case.
Thanks for spotting. Adding a test with invalid requirement which caused a crash.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140547/new/

https://reviews.llvm.org/D140547