Re: [PATCH] c++: call complete_type after performing auto deduction [PR80351]
On 3/29/22 02:02, Pokechu22 wrote:
On Thu, Mar 24, 2022 at 1:53 PM Jason Merrill wrote:
Thanks! For future reference, the patch doesn't apply easily because
gmail wrapped lines; for sending patches via gmail you'll need to use
attachments. Or you can use another MUA, or git send-email. This time
I fixed the wrapping by hand, but that's a pain (especially since
there's no "try again" flag to git am).
Thanks. I was vaguely aware that gmail did something wrong, but I
thought that enabling plain-text mode was enough to work around it.
It might be useful to add that information to the contributing
webpage. I've attached a copy of the patch here in case anyone else
wants to test it without dealing with the mangling.
Out of curiosity, do you know if this issue could have impacted
anything beyond the false warning? I had previously determined that
ensure_literal_type_for_constexpr_object completes the type, so for
constexpr variables it wouldn't have caused any further issues, but I
discovered that regular const variables were also affected later on
(and possibly other unused variables could be affected, but non-const
ones would trigger the warning normally).
It also affects whether the variable goes into .data or .rodata. For
#include
const auto x = { 1, 2 };
without the change, x goes into .data; after the change it goes in .rodata.
Also, like other DCO projects, we can't normally accept pseudonymous
contributions. But as you say in the PR, this patch is trivial enough
that I'm content to apply it myself; I want to make some adjustments anyway.
Also out of curiosity, what do you want to adjust with it? I'd like
to know in case it's relevant for any other patch that I submit
(though there probably won't be a situation where I'm writing another
patch for a while).
I'm adding a diagnostic if the deduced type remains incomplete, not
anything more generally relevant.
Lastly, I'd appreciate my pseudonym being credited (at least as a
co-author) in the final patch,
Yes, that was my plan.
Jason
Re: [PATCH] c++: call complete_type after performing auto deduction [PR80351]
On Thu, Mar 24, 2022 at 1:53 PM Jason Merrill wrote: > Thanks! For future reference, the patch doesn't apply easily because > gmail wrapped lines; for sending patches via gmail you'll need to use > attachments. Or you can use another MUA, or git send-email. This time > I fixed the wrapping by hand, but that's a pain (especially since > there's no "try again" flag to git am). Thanks. I was vaguely aware that gmail did something wrong, but I thought that enabling plain-text mode was enough to work around it. It might be useful to add that information to the contributing webpage. I've attached a copy of the patch here in case anyone else wants to test it without dealing with the mangling. Out of curiosity, do you know if this issue could have impacted anything beyond the false warning? I had previously determined that ensure_literal_type_for_constexpr_object completes the type, so for constexpr variables it wouldn't have caused any further issues, but I discovered that regular const variables were also affected later on (and possibly other unused variables could be affected, but non-const ones would trigger the warning normally). > Also, like other DCO projects, we can't normally accept pseudonymous > contributions. But as you say in the PR, this patch is trivial enough > that I'm content to apply it myself; I want to make some adjustments anyway. Also out of curiosity, what do you want to adjust with it? I'd like to know in case it's relevant for any other patch that I submit (though there probably won't be a situation where I'm writing another patch for a while). Lastly, I'd appreciate my pseudonym being credited (at least as a co-author) in the final patch, but I get that the patch being trivial enough that no credit is needed is why it's possible for it to be submitted pseudonymously in the first place, so if there's a policy that prohibits doing so that's fine. The more important thing is that the bug gets fixed :) --Poke > Currently GCC 12 development is in the regression fixes only stage, so > I'll queue this for GCC 13. > > Thanks again, > Jason > 0001-c-call-complete_type-after-performing-auto-deduction.patch Description: Binary data
Re: [PATCH] c++: call complete_type after performing auto deduction [PR80351]
On 3/23/22 21:01, Pokechu22 via Gcc-patches wrote: When cp_finish_decl calls cp_apply_type_quals_to_decl on a const auto or constexpr auto variable, the type might not be complete the first time (this happened when auto deduces to an initializer_list). cp_apply_type_quals_to_decl removes the const qualifier if the type is not complete, which is appropriate for grokdeclarator, on the assumption that the type will be complete when called by cp_finish_decl. Tested on x86_64 Ubuntu under WSL1 by bootstrapping and comparing results from 24d51e749570dcb85bd43d3b528f58ad6141de26 with results from this change. As far as I can tell, this fixes Wunused-var-38.C and Wunused-var-39.C without regressions. (Wunused-var-37.C passed before this change.) Thanks! For future reference, the patch doesn't apply easily because gmail wrapped lines; for sending patches via gmail you'll need to use attachments. Or you can use another MUA, or git send-email. This time I fixed the wrapping by hand, but that's a pain (especially since there's no "try again" flag to git am). Also, like other DCO projects, we can't normally accept pseudonymous contributions. But as you say in the PR, this patch is trivial enough that I'm content to apply it myself; I want to make some adjustments anyway. Currently GCC 12 development is in the regression fixes only stage, so I'll queue this for GCC 13. Thanks again, Jason
[PATCH] c++: call complete_type after performing auto deduction [PR80351]
When cp_finish_decl calls cp_apply_type_quals_to_decl on a const auto or
constexpr auto variable, the type might not be complete the first time
(this happened when auto deduces to an initializer_list).
cp_apply_type_quals_to_decl removes the const qualifier if the type is
not complete, which is appropriate for grokdeclarator, on the assumption
that the type will be complete when called by cp_finish_decl.
Tested on x86_64 Ubuntu under WSL1 by bootstrapping and comparing results
from 24d51e749570dcb85bd43d3b528f58ad6141de26 with results from this
change. As far as I can tell, this fixes Wunused-var-38.C and
Wunused-var-39.C without regressions.
(Wunused-var-37.C passed before this change.)
gcc/cp/ChangeLog:
PR c++/80351
* decl.cc (cp_finish_decl): Call complete_type after performing
auto deduction.
gcc/testsuite/ChangeLog:
PR c++/80351
* g++.dg/warn/Wunused-var-37.C: New test.
* g++.dg/warn/Wunused-var-38.C: New test.
* g++.dg/warn/Wunused-var-39.C: New test.
---
gcc/cp/decl.cc | 2 +-
gcc/testsuite/g++.dg/warn/Wunused-var-37.C | 64 ++
gcc/testsuite/g++.dg/warn/Wunused-var-38.C | 16 ++
gcc/testsuite/g++.dg/warn/Wunused-var-39.C | 16 ++
4 files changed, 97 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/warn/Wunused-var-37.C
create mode 100644 gcc/testsuite/g++.dg/warn/Wunused-var-38.C
create mode 100644 gcc/testsuite/g++.dg/warn/Wunused-var-39.C
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 34d9dad9fb0..e382b8ad218 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -8098,7 +8098,7 @@ cp_finish_decl (tree decl, tree init, bool
init_const_expr_p,
TREE_TYPE (decl) = error_mark_node;
return;
}
- cp_apply_type_quals_to_decl (cp_type_quals (type), decl);
+ cp_apply_type_quals_to_decl (cp_type_quals (complete_type (type)), decl);
}
if (ensure_literal_type_for_constexpr_object (decl) == error_mark_node)
diff --git a/gcc/testsuite/g++.dg/warn/Wunused-var-37.C
b/gcc/testsuite/g++.dg/warn/Wunused-var-37.C
new file mode 100644
index 000..54e76ac4e11
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wunused-var-37.C
@@ -0,0 +1,64 @@
+// Tangentially to PR c++/80351
+// { dg-do compile { target c++17 } }
+// { dg-options "-Wunused-variable" }
+#include
+
+// Warnings:
+static int int_s1 = 0; // { dg-warning "defined but not used" }
+static int int_s2 = 0; // { dg-warning "defined but not used" }
+inline static intint_is1 = 0; // { dg-warning "defined but not used" }
+inline static intint_is2 = 0; // { dg-warning "defined but not used" }
+// No warnings:
+constexpr static int int_cs1 = 0; // { dg-bogus "defined but not used" }
+constexpr static int int_cs2 = 0; // { dg-bogus "defined but not used" }
+int int_1 = 0; // { dg-bogus "defined but not used" }
+int int_2 = 0; // { dg-bogus "defined but not used" }
+inline int int_i1 = 0; // { dg-bogus "defined but not used" }
+inline int int_i2 = 0; // { dg-bogus "defined but not used" }
+constexpr intint_c1 = 0; // { dg-bogus "defined but not used" }
+constexpr intint_c2 = 0; // { dg-bogus "defined but not used" }
+
+// Warnings:
+static auto int_as1 = 0; // { dg-warning "defined but not used" }
+static auto int_as2 = 0; // { dg-warning "defined but not used" }
+inline static autoint_ais1 = 0; // { dg-warning "defined but not used" }
+inline static autoint_ais2 = 0; // { dg-warning "defined but not used" }
+// No warnings:
+constexpr static auto int_acs1 = 0; // { dg-bogus "defined but not used" }
+constexpr static auto int_acs2 = 0; // { dg-bogus "defined but not used" }
+auto int_a1 = 0; // { dg-bogus "defined but not used" }
+auto int_a2 = 0; // { dg-bogus "defined but not used" }
+inline auto int_ai1 = 0; // { dg-bogus "defined but not used" }
+inline auto int_ai2 = 0; // { dg-bogus "defined but not used" }
+constexpr autoint_ac1 = 0; // { dg-bogus "defined but not used" }
+constexpr autoint_ac2 = 0; // { dg-bogus "defined but not used" }
+
+// Warnings:
+static std::initializer_list il_s1 = {0, 1}; // {
dg-warning "defined but not used" }
+static std::initializer_list il_s2 = {0, 1}; // {
dg-warning "defined but not used" }
+inline static std::initializer_listil_is1 = {0, 1}; // {
dg-warning "defined but not used" }
+inline static std::initializer_listil_is2 = {0, 1}; // {
dg-warning "defined but not used" }
+// No warnings:
+constexpr static std::initializer_list il_cs1 = {0, 1}; // {
dg-bogus "defined but not used" }
+constexpr static std::initializer_list il_cs2 = {0, 1}; // {
dg-bogus "defined but not used" }
+std::initializer_list il_1 = {0, 1}; // {
dg-bogus "defined but not used" }
+std::initializer_list il_2 = {0, 1}; //
