[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno marked 2 inline comments as done.
riccibruno added inline comments.


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:91
+  auto CheckAndDiagnoseLocalEntity = [&](const VarDecl *VD, unsigned DiagID,
+                                         const auto &... DiagArgs) -> bool {
+    if (VD->isLocalVarDecl() && !DRE->isNonOdrUse()) {
----------------
aaronpuchert wrote:
> Since you're emitting a specific warning, I think you can just hardcode the expected types here instead of using `const auto&...`. Alternatively you could guess the integer argument from the (dynamic) type of the `VarDecl` argument.
I am using a pack here (changed a bit in the latest revision) to make it easy to re-use this lambda when the inconsistency between parameters and local variables is removed.


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:147
+    //
+    const auto *VD = cast_or_null<VarDecl>(Binding->getDecomposedDecl());
+    if (VD && CheckAndDiagnoseLocalEntity(
----------------
aaronpuchert wrote:
> Have you seen a case there the `_or_null` is relevant? To me it seems like this shouldn't happen, at least not when we get here.
I have removed it in the latest revision since we should fix the (de)serialization instead if this is possible.


================
Comment at: clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp:40
   extern void h6(int = i5);
-  // expected-error-re at -1 {{default argument references local variable '(unnamed variable of type (anonymous union at {{.*}}:20:3))' of enclosing function}}
+  // expected-error-re at -1 {{default argument references local variable '(unnamed variable of type (anonymous union at {{.*}}:35:3))' of enclosing function}}
+
----------------
aaronpuchert wrote:
> Better use a relative line number: `[[@LINE-5]]` or something like that.
I wish I could. Unfortunately `[[@LINE-5]]` is a `CHECK` line construct. Is it possible to refer to a relative line in a regex `verify` line?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85613