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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: lib/Sema/Sema.cpp:1677-1684
+Sema::DefaultArgRAII::DefaultArgRAII(Sema &S)
+    : Actions(S), PrevParent(S.getParentOfDefaultArg()) {
+  S.setParentOfDefaultArg(S.CurContext);
+}
+
+Sema::DefaultArgRAII::~DefaultArgRAII() {
+  Actions.setParentOfDefaultArg(PrevParent);
----------------
Any reason not to inline this in the header file?


================
Comment at: lib/Sema/SemaExpr.cpp:4523
+      // other parameters in unevaluated contexts.
+      if (FunctionDecl *Pattern = FD->getTemplateInstantiationPattern()) {
+        auto I = FD->param_begin();
----------------
`const FunctionDecl *`


================
Comment at: lib/Sema/SemaExpr.cpp:4535
+          unsigned NumExpansions =
+              *getNumArgumentsInExpansion(PVD->getType(), MutiLevelArgList);
+          CurrentInstantiationScope->MakeInstantiatedLocalArgPack(PVD);
----------------
Is it possible for the `Optional<>` returned here to not hold a value?


================
Comment at: lib/Sema/SemaExpr.cpp:13746-13751
+    unsigned DiagID;
+    if (isa<ParmVarDecl>(var))
+      DiagID = diag::err_param_default_argument_references_param;
+    else
+      DiagID = diag::err_param_default_argument_references_local;
+    S.Diag(loc, DiagID) << var->getDeclName();
----------------
Ternary operator in `S.Diag()` would be just as clear, I think.

Also, you should not need to call `getDeclName()` as the diagnostic engine will automatically do the right thing. As is stands, this won't properly quote the name in the diagnostic.


https://reviews.llvm.org/D36915