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

Aaron Puchert via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 2 09:27:40 PDT 2020


aaronpuchert 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()) {
----------------
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.


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:147
+    //
+    const auto *VD = cast_or_null<VarDecl>(Binding->getDecomposedDecl());
+    if (VD && CheckAndDiagnoseLocalEntity(
----------------
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.


================
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}}
+
----------------
Better use a relative line number: `[[@LINE-5]]` or something like that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85613



More information about the cfe-commits mailing list