[PATCH] D36915: [Sema] Diagnose local variables and parameters captured by lambda and block expressions in a default argument

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Please take a look at http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0588r1.html, which respecifies the rules for lambda capture and its interaction with default arguments, and has been voted into the C++ working paper as a defect report resolution. The approach there is to use a purely syntactic, scope-based mechanism to detect problems such as this. (In dependent contexts, we can track on the `DeclRefExpr` whether the name is odr-usable, in case we can't tell whether it's odr-used from the template definition alone.)



================
Comment at: include/clang/Sema/Sema.h:1062
+
+  const DeclContext *ParentOfDefaultArg = nullptr;
+
----------------
There are lots of cases where we switch context in the middle of handling an expression, for instance to instantiate a template (or even *parse* a template in MSVC-compatible delayed template parsing mode). It's not reasonable to add a new form of state that all those places will need to save and restore themselves.

Please consider whether this would make sense as a member of the `ExpressionEvaluationContextRecord` or similar.


================
Comment at: lib/Sema/SemaExpr.cpp:4520-4541
+      // Add mappings for instantiated parameters appearing before Param. This
+      // is needed to instantiate default argument expressions referencing
+      // other parameters in unevaluated contexts.
+      if (FunctionDecl *Pattern = FD->getTemplateInstantiationPattern()) {
+        auto I = FD->param_begin();
+        for (const auto *PVD : Pattern->parameters()) {
+          if (*I == Param)
----------------
Use `addInstantiatedParametersToScope` for this.

This bugfix looks to be independent of the fix for lambdas; can you factor it out into a separate patch?


https://reviews.llvm.org/D36915