[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.
aaronpuchert added a comment. Ah, I guess Ianded on an older version of this through a link and didn't notice. Nevermind. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85613/new/ https://reviews.llvm.org/D85613 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.
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(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@-1 {{default argument references local variable
'(unnamed variable of type (anonymous union at {{.*}}:20:3))' of enclosing
function}}
+ // expected-error-re@-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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.
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(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@-1 {{default argument references local variable
'(unnamed variable of type (anonymous union at {{.*}}:20:3))' of enclosing
function}}
+ // expected-error-re@-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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.
riccibruno updated this revision to Diff 284803.
riccibruno added a comment.
Remove the now-unused `const VarDecl *` parameter to `DiagnoseIfOdrUse`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85613/new/
https://reviews.llvm.org/D85613
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
Index: clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
===
--- clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
+++ clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
@@ -1,5 +1,20 @@
// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+namespace std {
+using size_t = decltype(sizeof(int));
+template struct tuple_size;
+template struct tuple_element;
+} // namespace std
+using size_t = std::size_t;
+
+struct E {};
+template int get(E);
+
+namespace std {
+template <> struct tuple_size { static constexpr size_t value = 2; };
+template struct tuple_element { using type = int; };
+}; // namespace std
+
void h() {
int i1 = 0;
extern void h1(int x = i1);
@@ -24,8 +39,19 @@
extern void h6(int = i5);
// expected-error@-1 {{default argument references local variable '' of enclosing function}}
- struct S { int i; };
- auto [x] = S();
+ struct S {
+int i, j;
+ };
+ auto [x, y] = S();
+
+ extern void h7(int = x); // expected-error {{default argument references local binding 'x'}}
+
+ auto [z, w] = E();
+ extern void h8a(int = sizeof(z)); // ok
+ extern void h8b(int = w); // expected-error {{default argument references local binding 'w'}}
- extern void h7(int = x); // FIXME: reject
+ extern auto get_array()->int(&)[2];
+ auto [a0, a1] = get_array();
+ extern void h9a(int = sizeof(a0));
+ extern void h9b(int = a1); // expected-error {{default argument references local binding 'a1'}}
}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -86,6 +86,20 @@
/// argument expression.
bool CheckDefaultArgumentVisitor::VisitDeclRefExpr(const DeclRefExpr *DRE) {
const NamedDecl *Decl = DRE->getDecl();
+
+ auto DiagnoseIfOdrUse = [&](unsigned DiagID,
+ const auto &... DiagArgs) -> bool {
+if (!DRE->isNonOdrUse()) {
+ auto DB = S.Diag(DRE->getBeginLoc(), DiagID);
+ int dummy[] = {0, (DB << DiagArgs, 0)...};
+ (void)dummy;
+ DB << DefaultArg->getSourceRange();
+ return true;
+}
+
+return false;
+ };
+
if (const auto *Param = dyn_cast(Decl)) {
// C++ [dcl.fct.default]p9:
// [...] parameters of a function shall not be used in default
@@ -111,10 +125,28 @@
// C++20 [dcl.fct.default]p7 (DR as part of P0588R1, see also CWG 2346):
// Note: A local variable cannot be odr-used (6.3) in a default argument.
//
-if (VDecl->isLocalVarDecl() && !DRE->isNonOdrUse())
- return S.Diag(DRE->getBeginLoc(),
-diag::err_param_default_argument_references_local)
- << VDecl->getDeclName() << DefaultArg->getSourceRange();
+if (VDecl->isLocalVarDecl() &&
+DiagnoseIfOdrUse(diag::err_param_default_argument_references_local,
+ /*variable*/ 0, VDecl))
+ return true;
+
+ } else if (const auto *Binding = dyn_cast(Decl)) {
+// C++20 [basic.pre]p7:
+// A local entity is [...] a structured binding whose corresponding
+// variable is such an entity [...]
+//
+// C++20 [basic.def.odr]p9:
+// A local entity (6.1) is odr-usable in a declarative region if [...]
+//
+// Note that this was not entirely clear in C++17 since [dcl.fct.default]p7
+// only prohibited local variables (a structured binding declaration
+// introduces identifiers as names).
+//
+const auto *VD = cast(Binding->getDecomposedDecl());
+if (VD->isLocalVarDecl() &&
+DiagnoseIfOdrUse(diag::err_param_default_argument_references_local,
+ /*binding*/ 1, Binding))
+ return true;
}
return false;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3994,7 +3994,8 @@
def err_param_default_argument_references_param : Error<
"default argument references parameter %0">;
def err_param_default_argument_references_local : Error<
- "default argument references local variable %0 of enclosing function">;
+ "default argument references local %select{variable|binding}0 %1 "
+ "of enclosing function">;
def err_param_default_argument_references_this : Error<
"default argument references 'this'">;
def err_param_default_argument_nonfunc : Error<
[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.
riccibruno added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:112 // if (DRE->isNonOdrUse() != NOUR_Unevaluated) return S.Diag(DRE->getBeginLoc(), This can use `DiagnoseIfOdrUse` as soon as the inconsistency between parameters and local variables is removed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85613/new/ https://reviews.llvm.org/D85613 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.
riccibruno added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:3843 /// The declaration that this binding binds to part of. + // FIXME: Currently not set during deserialization of the BindingDecl; + // only set when the corresponding DecompositionDecl is visited. rsmith wrote: > !! This seems pretty bad; would it be hard to fix? On further examination, I believe that`Decomp` is set but this is subtle, and it is likely that I am missing a case/wrong somehow. The expression for the binding (`Binding`) will be deserialized when visiting the `BindingDecl`. This expression when non-null will always (as far as I can tell) contain a reference to the decomposition declaration so the decomposition will be deserialized, which will set `Decomp`. The binding expression is null when the initializer is type-dependent. But then, since variable template decompositions are not allowed, the decomposition must occurs at block scope (`mayHaveDecompositionDeclarator`). This means that the `DecompositionDecl` will be read first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85613/new/ https://reviews.llvm.org/D85613 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.
riccibruno updated this revision to Diff 284718.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85613/new/
https://reviews.llvm.org/D85613
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
Index: clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
===
--- clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
+++ clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
@@ -1,5 +1,20 @@
// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+namespace std {
+using size_t = decltype(sizeof(int));
+template struct tuple_size;
+template struct tuple_element;
+} // namespace std
+using size_t = std::size_t;
+
+struct E {};
+template int get(E);
+
+namespace std {
+template <> struct tuple_size { static constexpr size_t value = 2; };
+template struct tuple_element { using type = int; };
+}; // namespace std
+
void h() {
int i1 = 0;
extern void h1(int x = i1);
@@ -24,8 +39,19 @@
extern void h6(int = i5);
// expected-error@-1 {{default argument references local variable '' of enclosing function}}
- struct S { int i; };
- auto [x] = S();
+ struct S {
+int i, j;
+ };
+ auto [x, y] = S();
+
+ extern void h7(int = x); // expected-error {{default argument references local binding 'x'}}
+
+ auto [z, w] = E();
+ extern void h8a(int = sizeof(z)); // ok
+ extern void h8b(int = w); // expected-error {{default argument references local binding 'w'}}
- extern void h7(int = x); // FIXME: reject
+ extern auto get_array()->int(&)[2];
+ auto [a0, a1] = get_array();
+ extern void h9a(int = sizeof(a0));
+ extern void h9b(int = a1); // expected-error {{default argument references local binding 'a1'}}
}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -86,6 +86,20 @@
/// argument expression.
bool CheckDefaultArgumentVisitor::VisitDeclRefExpr(const DeclRefExpr *DRE) {
const NamedDecl *Decl = DRE->getDecl();
+
+ auto DiagnoseIfOdrUse = [&](const VarDecl *VD, unsigned DiagID,
+ const auto &... DiagArgs) -> bool {
+if (!DRE->isNonOdrUse()) {
+ auto DB = S.Diag(DRE->getBeginLoc(), DiagID);
+ int dummy[] = {0, (DB << DiagArgs, 0)...};
+ (void)dummy;
+ DB << DefaultArg->getSourceRange();
+ return true;
+}
+
+return false;
+ };
+
if (const auto *Param = dyn_cast(Decl)) {
// C++ [dcl.fct.default]p9:
// [...] parameters of a function shall not be used in default
@@ -111,10 +125,29 @@
// C++20 [dcl.fct.default]p7 (DR as part of P0588R1, see also CWG 2346):
// Note: A local variable cannot be odr-used (6.3) in a default argument.
//
-if (VDecl->isLocalVarDecl() && !DRE->isNonOdrUse())
- return S.Diag(DRE->getBeginLoc(),
-diag::err_param_default_argument_references_local)
- << VDecl->getDeclName() << DefaultArg->getSourceRange();
+if (VDecl->isLocalVarDecl() &&
+DiagnoseIfOdrUse(VDecl,
+ diag::err_param_default_argument_references_local,
+ /*variable*/ 0, VDecl))
+ return true;
+
+ } else if (const auto *Binding = dyn_cast(Decl)) {
+// C++20 [basic.pre]p7:
+// A local entity is [...] a structured binding whose corresponding
+// variable is such an entity [...]
+//
+// C++20 [basic.def.odr]p9:
+// A local entity (6.1) is odr-usable in a declarative region if [...]
+//
+// Note that this was not entirely clear in C++17 since [dcl.fct.default]p7
+// only prohibited local variables (a structured binding declaration
+// introduces identifiers as names).
+//
+const auto *VD = cast(Binding->getDecomposedDecl());
+if (VD->isLocalVarDecl() &&
+DiagnoseIfOdrUse(VD, diag::err_param_default_argument_references_local,
+ /*binding*/ 1, Binding))
+ return true;
}
return false;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3994,7 +3994,8 @@
def err_param_default_argument_references_param : Error<
"default argument references parameter %0">;
def err_param_default_argument_references_local : Error<
- "default argument references local variable %0 of enclosing function">;
+ "default argument references local %select{variable|binding}0 %1 "
+ "of enclosing function">;
def err_param_default_argument_references_this : Error<
"default argument references 'this'">;
def err_param_default_argument_nonfunc : Error<
___
[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.
rsmith added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:3843 /// The declaration that this binding binds to part of. + // FIXME: Currently not set during deserialization of the BindingDecl; + // only set when the corresponding DecompositionDecl is visited. !! This seems pretty bad; would it be hard to fix? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85613/new/ https://reviews.llvm.org/D85613 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.
riccibruno added inline comments.
Comment at: clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp:51
+ extern void h8a(int = sizeof(z)); // ok
+ extern void h8b(int = w); // expected-error {{default argument
references local variable 'w'}}
rsmith wrote:
> The diagnostic in this case is inconsistent with the non-tuple-like cases. I
> think this diagnostic is better; we should use the original `Decl` when
> producing the diagnostic, not the decomposed variable.
Right, let's just refer to the binding in all cases.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85613/new/
https://reviews.llvm.org/D85613
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.
riccibruno updated this revision to Diff 284364.
riccibruno marked 2 inline comments as done.
riccibruno edited the summary of this revision.
riccibruno added a comment.
Refer to the binding in the diagnostic.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85613/new/
https://reviews.llvm.org/D85613
Files:
clang/include/clang/AST/DeclCXX.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
Index: clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
===
--- clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
+++ clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
@@ -1,5 +1,20 @@
// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+namespace std {
+using size_t = decltype(sizeof(int));
+template struct tuple_size;
+template struct tuple_element;
+} // namespace std
+using size_t = std::size_t;
+
+struct E {};
+template int get(E);
+
+namespace std {
+template <> struct tuple_size { static constexpr size_t value = 2; };
+template struct tuple_element { using type = int; };
+}; // namespace std
+
void h() {
int i1 = 0;
extern void h1(int x = i1);
@@ -22,10 +37,21 @@
};
extern void h6(int = i5);
- // expected-error-re@-1 {{default argument references local variable '(unnamed variable of type (anonymous union at {{.*}}:20:3))' of enclosing function}}
+ // expected-error-re@-1 {{default argument references local variable '(unnamed variable of type (anonymous union at {{.*}}:35:3))' of enclosing function}}
+
+ struct S {
+int i, j;
+ };
+ auto [x, y] = S();
+
+ extern void h7(int = x); // expected-error {{default argument references local binding 'x'}}
- struct S { int i; };
- auto [x] = S();
+ auto [z, w] = E();
+ extern void h8a(int = sizeof(z)); // ok
+ extern void h8b(int = w); // expected-error {{default argument references local binding 'w'}}
- extern void h7(int = x); // FIXME: reject
+ extern auto get_array()->int(&)[2];
+ auto [a0, a1] = get_array();
+ extern void h9a(int = sizeof(a0));
+ extern void h9b(int = a1); // expected-error {{default argument references local binding 'a1'}}
}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -86,6 +86,22 @@
/// argument expression.
bool CheckDefaultArgumentVisitor::VisitDeclRefExpr(const DeclRefExpr *DRE) {
const NamedDecl *Decl = DRE->getDecl();
+
+ auto CheckAndDiagnoseLocalEntity = [&](const VarDecl *VD, unsigned DiagID,
+ const auto &... DiagArgs) -> bool {
+if (VD->isLocalVarDecl() && !DRE->isNonOdrUse()) {
+ auto DB = S.Diag(DRE->getBeginLoc(), DiagID);
+ // FIXME: Replace with a fold once we can use C++17.
+ int dummy[] = {(DB << DiagArgs, 0)...};
+ (void)dummy;
+
+ DB << DefaultArg->getSourceRange();
+ return true;
+}
+
+return false;
+ };
+
if (const auto *Param = dyn_cast(Decl)) {
// C++ [dcl.fct.default]p9:
// [...] parameters of a function shall not be used in default
@@ -111,10 +127,28 @@
// C++20 [dcl.fct.default]p7 (DR as part of P0588R1, see also CWG 2346):
// Note: A local variable cannot be odr-used (6.3) in a default argument.
//
-if (VDecl->isLocalVarDecl() && !DRE->isNonOdrUse())
- return S.Diag(DRE->getBeginLoc(),
-diag::err_param_default_argument_references_local)
- << VDecl << DefaultArg->getSourceRange();
+if (CheckAndDiagnoseLocalEntity(
+VDecl, diag::err_param_default_argument_references_local,
+/*variable*/ 0, VDecl))
+ return true;
+
+ } else if (const auto *Binding = dyn_cast(Decl)) {
+// C++20 [basic.pre]p7:
+// A local entity is [...] a structured binding whose corresponding
+// variable is such an entity [...]
+//
+// C++20 [basic.def.odr]p9:
+// A local entity (6.1) is odr-usable in a declarative region if [...]
+//
+// Note that this was not entirely clear in C++17 since [dcl.fct.default]p7
+// only prohibited local variables (a structured binding declaration
+// introduces identifiers as names).
+//
+const auto *VD = cast_or_null(Binding->getDecomposedDecl());
+if (VD && CheckAndDiagnoseLocalEntity(
+ VD, diag::err_param_default_argument_references_local,
+ /*binding*/ 1, Binding))
+ return true;
}
return false;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3988,7 +3988,8 @@
def err_param_default_argument_references_param : Error<
[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.
rsmith added inline comments.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:101-105
+if (const VarDecl *HoldingVar = Binding->getHoldingVar()) {
+ // C++20 [dcl.struct.bind]p4:
+ // Each vi is the name [...] that refers to the object bound to ri
[...]
+ Decl = HoldingVar;
+} else {
Is there a reason to separate these two cases? (Could we just use the
decomposed decl unconditionally?) Generally treating tuple-like decompositions
differently from other kinds seems error-prone.
Comment at: clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp:51
+ extern void h8a(int = sizeof(z)); // ok
+ extern void h8b(int = w); // expected-error {{default argument
references local variable 'w'}}
The diagnostic in this case is inconsistent with the non-tuple-like cases. I
think this diagnostic is better; we should use the original `Decl` when
producing the diagnostic, not the decomposed variable.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85613/new/
https://reviews.llvm.org/D85613
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.
riccibruno created this revision.
riccibruno added reviewers: rsmith, erichkeane, rjmccall.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.
riccibruno requested review of this revision.
Currently clang accepts a default argument containing a structured binding
which is a local entity:
struct S { int i, j; };
auto [x, y] = S();
extern void h(int = x); // Should be rejected.
It was not entirely clear in C++17 that this was forbidden since `C++17
[dcl.fct.default]p7` only used the term "local variable". However this is
clearly forbidden in C++20 with the new wording in terms of odr-usable local
entities.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D85613
Files:
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
Index: clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
===
--- clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
+++ clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
@@ -1,5 +1,20 @@
// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+namespace std {
+using size_t = decltype(sizeof(int));
+template struct tuple_size;
+template struct tuple_element;
+} // namespace std
+using size_t = std::size_t;
+
+struct E {};
+template int get(E);
+
+namespace std {
+template <> struct tuple_size { static constexpr size_t value = 2; };
+template struct tuple_element { using type = int; };
+}; // namespace std
+
void h() {
int i1 = 0;
extern void h1(int x = i1);
@@ -22,10 +37,21 @@
};
extern void h6(int = i5);
- // expected-error-re@-1 {{default argument references local variable
'(unnamed variable of type (anonymous union at {{.*}}:20:3))' of enclosing
function}}
+ // expected-error-re@-1 {{default argument references local variable
'(unnamed variable of type (anonymous union at {{.*}}:35:3))' of enclosing
function}}
+
+ struct S {
+int i, j;
+ };
+ auto [x, y] = S();
+
+ extern void h7(int = x); // expected-error {{default argument references
local variable '[x, y]'}}
- struct S { int i; };
- auto [x] = S();
+ auto [z, w] = E();
+ extern void h8a(int = sizeof(z)); // ok
+ extern void h8b(int = w); // expected-error {{default argument
references local variable 'w'}}
- extern void h7(int = x); // FIXME: reject
+ extern auto get_array()->int(&)[2];
+ auto [a0, a1] = get_array();
+ extern void h9a(int = sizeof(a0));
+ extern void h9b(int = a1); // expected-error {{default argument references
local variable '[a0, a1]'}}
}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -86,6 +86,34 @@
/// argument expression.
bool CheckDefaultArgumentVisitor::VisitDeclRefExpr(const DeclRefExpr *DRE) {
const NamedDecl *Decl = DRE->getDecl();
+
+ // C++20 [basic.pre]p7:
+ // A local entity is [...] a structured binding whose corresponding
variable
+ // is such an entity [...]
+ //
+ // C++20 [basic.def.odr]p9:
+ // A local entity (6.1) is odr-usable in a declarative region if [...]
+ //
+ // Note that this was not entirely clear in C++17 since [dcl.fct.default]p7
+ // only prohibited local variables (a structured binding declaration
+ // introduces identifiers as names).
+ if (const auto *Binding = dyn_cast(Decl)) {
+if (const VarDecl *HoldingVar = Binding->getHoldingVar()) {
+ // C++20 [dcl.struct.bind]p4:
+ // Each vi is the name [...] that refers to the object bound to ri
[...]
+ Decl = HoldingVar;
+} else {
+ // C++20 [dcl.struct.bind]p3:
+ // Each vi is the name of an lvalue that refers to the element i of
+ // the array [...]
+ //
+ // C++20 [dcl.struct.bind]p5:
+ // [...] each vi is the name of an lvalue that refers to the member mi
+ // of e [...]
+ Decl = Binding->getDecomposedDecl();
+}
+ }
+
if (const auto *Param = dyn_cast(Decl)) {
// C++ [dcl.fct.default]p9:
// [...] parameters of a function shall not be used in default
Index: clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
===
--- clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
+++ clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
@@ -1,5 +1,20 @@
// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+namespace std {
+using size_t = decltype(sizeof(int));
+template struct tuple_size;
+template struct tuple_element;
+} // namespace std
+using size_t = std::size_t;
+
+struct E {};
+template int get(E);
+
+namespace std {
+template <> struct tuple_size { static constexpr size_t value = 2; };
+template struct tuple_element { using type = int; };
+}; // namespace std
+
void h() {
int i1 = 0;
extern void h1(int x = i1);
@@ -22,10 +37,21 @@
};
extern void
