Improve diagnostics for invalid use of void #2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
'void value not ignored as it ought to be' is a holdover from K&R C and is utterly incomprehensible to the modern C++ developer. Switch to 'invalid use of void expression' (already in use in cp/call.cc) and add context/expected type information where available.
Spell the void cst as 'void()' per C++ (e.g. [dcl.type.auto.deduct], [temp.variadic])
ecatmur
pushed a commit
that referenced
this pull request
Jul 17, 2022
Here during cp_parser_single_declaration for #2, we were calling associate_classtype_constraints for TPL<T> (the primary template type) before maybe_process_partial_specialization could get a chance to notice that we're in fact declaring a distinct constrained partial spec and not redeclaring the primary template. This caused us to emit a bogus error about differing constraints b/t the primary template and #2's constraints. This patch fixes this by moving the call to associate_classtype_constraints after the call to shadow_tag (which calls maybe_process_partial_specialization) and adjusting shadow_tag to use the return value of m_p_p_s. Moreover, if we later try to define a constrained partial specialization that's been declared earlier (as in the third testcase), then maybe_new_partial_specialization correctly notices it's a redeclaration and returns NULL_TREE. But in this case we also need to update TYPE to point to the redeclared partial spec (it'll otherwise continue pointing to the primary template type, eventually leading to a bogus error). PR c++/96363 gcc/cp/ChangeLog: * decl.cc (shadow_tag): Use the return value of maybe_process_partial_specialization. * parser.cc (cp_parser_single_declaration): Call shadow_tag before associate_classtype_constraints. * pt.cc (maybe_new_partial_specialization): Change return type to bool. Take 'type' argument by mutable reference. Set 'type' to point to the correct constrained specialization when appropriate. (maybe_process_partial_specialization): Adjust accordingly. gcc/testsuite/ChangeLog: * g++.dg/cpp2a/concepts-partial-spec12.C: New test. * g++.dg/cpp2a/concepts-partial-spec12a.C: New test. * g++.dg/cpp2a/concepts-partial-spec13.C: New test.
ecatmur
pushed a commit
that referenced
this pull request
Jul 17, 2022
As explained in r11-4959-gde6f64f9556ae3, the atom cache assumes two equivalent expressions (according to cp_tree_equal) must use the same template parameters (according to find_template_parameters). This assumption turned out to not hold for TARGET_EXPR, which was addressed by that commit. But this assumption apparently doesn't hold for PARM_DECL either: find_template_parameters walks its DECL_CONTEXT but cp_tree_equal by default doesn't consider DECL_CONTEXT unless comparing_specializations is set. Thus in the first testcase below, the atomic constraints of #1 and #2 are equivalent according to cp_tree_equal, but according to find_template_parameters the former uses T and the latter uses both T and U (surprisingly). We could fix this assumption violation by setting comparing_specializations in the atom_hasher, which would make cp_tree_equal return false for the two atoms, but that seems overly pessimistic here. Ideally the atoms should continue being considered equivalent and we instead fix find_template_paremeters to return just T for #2's atom. To that end this patch makes for_each_template_parm_r stop walking the DECL_CONTEXT of a PARM_DECL. This should be safe to do because tsubst_copy / tsubst_decl only substitutes the TREE_TYPE of a PARM_DECL and doesn't bother substituting the DECL_CONTEXT, thus the only relevant template parameters are those used in its type. any_template_parm_r is currently responsible for walking its TREE_TYPE, but I suppose it now makes sense for for_each_template_parm_r to do so instead. In passing this patch also makes for_each_template_parm_r stop walking the DECL_CONTEXT of a VAR_/FUNCTION_DECL since doing so after walking DECL_TI_ARGS is redundant, I think. I experimented with not walking DECL_CONTEXT for CONST_DECL, but the second testcase below demonstrates it's necessary to walk it. PR c++/105797 gcc/cp/ChangeLog: * pt.cc (for_each_template_parm_r) <case FUNCTION_DECL, VAR_DECL>: Don't walk DECL_CONTEXT. <case PARM_DECL>: Likewise. Walk TREE_TYPE. <case CONST_DECL>: Simplify. (any_template_parm_r) <case PARM_DECL>: Don't walk TREE_TYPE. gcc/testsuite/ChangeLog: * g++.dg/cpp2a/concepts-decltype4.C: New test. * g++.dg/cpp2a/concepts-memfun3.C: New test.
ecatmur
pushed a commit
that referenced
this pull request
Jul 17, 2022
This patch implements C++23 P2255R2, which adds two new type traits to
detect reference binding to a temporary. They can be used to detect code
like
std::tuple<const std::string&> t("meow");
which is incorrect because it always creates a dangling reference, because
the std::string temporary is created inside the selected constructor of
std::tuple, and not outside it.
There are two new compiler builtins, __reference_constructs_from_temporary
and __reference_converts_from_temporary. The former is used to simulate
direct- and the latter copy-initialization context. But I had a hard time
finding a test where there's actually a difference. Under DR 2267, both
of these are invalid:
struct A { } a;
struct B { explicit B(const A&); };
const B &b1{a};
const B &b2(a);
so I had to peruse [over.match.ref], and eventually realized that the
difference can be seen here:
struct G {
operator int(); // #1
explicit operator int&&(); // #2
};
int&& r1(G{}); // use #2 (no temporary)
int&& r2 = G{}; // use #1 (a temporary is created to be bound to int&&)
The implementation itself was rather straightforward because we already
have the conv_binds_ref_to_prvalue function. The main function here is
ref_xes_from_temporary.
I've changed the return type of ref_conv_binds_directly to tristate, because
previously the function didn't distinguish between an invalid conversion and
one that binds to a prvalue. Since it no longer returns a bool, I removed
the _p suffix.
The patch also adds the relevant class and variable templates to <type_traits>.
PR c++/104477
gcc/c-family/ChangeLog:
* c-common.cc (c_common_reswords): Add
__reference_constructs_from_temporary and
__reference_converts_from_temporary.
* c-common.h (enum rid): Add RID_REF_CONSTRUCTS_FROM_TEMPORARY and
RID_REF_CONVERTS_FROM_TEMPORARY.
gcc/cp/ChangeLog:
* call.cc (ref_conv_binds_directly_p): Rename to ...
(ref_conv_binds_directly): ... this. Add a new bool parameter. Change
the return type to tristate.
* constraint.cc (diagnose_trait_expr): Handle
CPTK_REF_CONSTRUCTS_FROM_TEMPORARY and CPTK_REF_CONVERTS_FROM_TEMPORARY.
* cp-tree.h: Include "tristate.h".
(enum cp_trait_kind): Add CPTK_REF_CONSTRUCTS_FROM_TEMPORARY
and CPTK_REF_CONVERTS_FROM_TEMPORARY.
(ref_conv_binds_directly_p): Rename to ...
(ref_conv_binds_directly): ... this.
(ref_xes_from_temporary): Declare.
* cxx-pretty-print.cc (pp_cxx_trait_expression): Handle
CPTK_REF_CONSTRUCTS_FROM_TEMPORARY and CPTK_REF_CONVERTS_FROM_TEMPORARY.
* method.cc (ref_xes_from_temporary): New.
* parser.cc (cp_parser_primary_expression): Handle
RID_REF_CONSTRUCTS_FROM_TEMPORARY and RID_REF_CONVERTS_FROM_TEMPORARY.
(cp_parser_trait_expr): Likewise.
(warn_for_range_copy): Adjust to call ref_conv_binds_directly.
* semantics.cc (trait_expr_value): Handle
CPTK_REF_CONSTRUCTS_FROM_TEMPORARY and CPTK_REF_CONVERTS_FROM_TEMPORARY.
(finish_trait_expr): Likewise.
libstdc++-v3/ChangeLog:
* include/std/type_traits (reference_constructs_from_temporary,
reference_converts_from_temporary): New class templates.
(reference_constructs_from_temporary_v,
reference_converts_from_temporary_v): New variable templates.
(__cpp_lib_reference_from_temporary): Define for C++23.
* include/std/version (__cpp_lib_reference_from_temporary): Define for
C++23.
* testsuite/20_util/variable_templates_for_traits.cc: Test
reference_constructs_from_temporary_v and
reference_converts_from_temporary_v.
* testsuite/20_util/reference_from_temporary/value.cc: New test.
* testsuite/20_util/reference_from_temporary/value2.cc: New test.
* testsuite/20_util/reference_from_temporary/version.cc: New test.
gcc/testsuite/ChangeLog:
* g++.dg/ext/reference_constructs_from_temporary1.C: New test.
* g++.dg/ext/reference_converts_from_temporary1.C: New test.
ecatmur
pushed a commit
that referenced
this pull request
Aug 19, 2022
This patch implements some additional zero-extension and sign-extension related optimizations in simplify-rtx.cc. The original motivation comes from PR rtl-optimization/71775, where in comment #2 Andrew Pinksi sees: Failed to match this instruction: (set (reg:DI 88 [ _1 ]) (sign_extend:DI (subreg:SI (ctz:DI (reg/v:DI 86 [ x ])) 0))) On many platforms the result of DImode CTZ is constrained to be a small unsigned integer (between 0 and 64), hence the truncation to 32-bits (using a SUBREG) and the following sign extension back to 64-bits are effectively a no-op, so the above should ideally (often) be simplified to "(set (reg:DI 88) (ctz:DI (reg/v:DI 86 [ x ]))". To implement this, and some closely related transformations, we build upon the existing val_signbit_known_clear_p predicate. In the first chunk, nonzero_bits knows that FFS and ABS can't leave the sign-bit bit set, so the simplification of of ABS (ABS (x)) and ABS (FFS (x)) can itself be simplified. The second transformation is that we can canonicalized SIGN_EXTEND to ZERO_EXTEND (as in the PR 71775 case above) when the operand's sign-bit is known to be clear. The final two chunks are for SIGN_EXTEND of a truncating SUBREG, and ZERO_EXTEND of a truncating SUBREG respectively. The nonzero_bits of a truncating SUBREG pessimistically thinks that the upper bits may have an arbitrary value (by taking the SUBREG), so we need look deeper at the SUBREG's operand to confirm that the high bits are known to be zero. Unfortunately, for PR rtl-optimization/71775, ctz:DI on x86_64 with default architecture options is undefined at zero, so we can't be sure the upper bits of reg:DI 88 will be sign extended (all zeros or all ones). nonzero_bits knows this, so the above transformations don't trigger, but the transformations themselves are perfectly valid for other operations such as FFS, POPCOUNT and PARITY, and on other targets/-march settings where CTZ is defined at zero. 2022-08-03 Roger Sayle <roger@nextmovesoftware.com> Segher Boessenkool <segher@kernel.crashing.org> Richard Sandiford <richard.sandiford@arm.com> gcc/ChangeLog * simplify-rtx.cc (simplify_unary_operation_1) <ABS>: Add optimizations for CLRSB, PARITY, POPCOUNT, SS_ABS and LSHIFTRT that are all positive to complement the existing FFS and idempotent ABS simplifications. <SIGN_EXTEND>: Canonicalize SIGN_EXTEND to ZERO_EXTEND when val_signbit_known_clear_p is true of the operand. Simplify sign extensions of SUBREG truncations of operands that are already suitably (zero) extended. <ZERO_EXTEND>: Simplify zero extensions of SUBREG truncations of operands that are already suitably zero extended.
ecatmur
pushed a commit
that referenced
this pull request
Aug 19, 2022
In my previous patches I've been extending our std::move warnings,
but this tweak actually dials it down a little bit. As reported in
bug 89780, it's questionable to warn about expressions in templates
that were type-dependent, but aren't anymore because we're instantiating
the template. As in,
template <typename T>
Dest withMove() {
T x;
return std::move(x);
}
template Dest withMove<Dest>(); // #1
template Dest withMove<Source>(); // #2
Saying that the std::move is pessimizing for #1 is not incorrect, but
it's not useful, because removing the std::move would then pessimize #2.
So the user can't really win. At the same time, disabling the warning
just because we're in a template would be going too far, I still want to
warn for
template <typename>
Dest withMove() {
Dest x;
return std::move(x);
}
because the std::move therein will be pessimizing for any instantiation.
So I'm using the suppress_warning machinery to that effect.
Problem: I had to add a new group to nowarn_spec_t, otherwise
suppressing the -Wpessimizing-move warning would disable a whole bunch
of other warnings, which we really don't want.
PR c++/89780
gcc/cp/ChangeLog:
* pt.cc (tsubst_copy_and_build) <case CALL_EXPR>: Maybe suppress
-Wpessimizing-move.
* typeck.cc (maybe_warn_pessimizing_move): Don't issue warnings
if they are suppressed.
(check_return_expr): Disable -Wpessimizing-move when returning
a dependent expression.
gcc/ChangeLog:
* diagnostic-spec.cc (nowarn_spec_t::nowarn_spec_t): Handle
OPT_Wpessimizing_move and OPT_Wredundant_move.
* diagnostic-spec.h (nowarn_spec_t): Add NW_REDUNDANT enumerator.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/Wpessimizing-move3.C: Remove dg-warning.
* g++.dg/cpp0x/Wredundant-move2.C: Likewise.
ecatmur
pushed a commit
that referenced
this pull request
Feb 4, 2023
While looking at PR 105549, which is about fixing the ABI break introduced in GCC 9.1 in parameter alignment with bit-fields, we noticed that the GCC 9.1 warning is not emitted in all the cases where it should be. This patch fixes that and the next patch in the series fixes the GCC 9.1 break. We split this into two patches since patch #2 introduces a new ABI break starting with GCC 13.1. This way, patch #1 can be back-ported to release branches if needed to fix the GCC 9.1 warning issue. The main idea is to add a new global boolean that indicates whether we're expanding the start of a function, so that aarch64_layout_arg can emit warnings for callees as well as callers. This removes the need for aarch64_function_arg_boundary to warn (with its incomplete information). However, in the first patch there are still cases where we emit warnings were we should not; this is fixed in patch #2 where we can distinguish between GCC 9.1 and GCC.13.1 ABI breaks properly. The fix in aarch64_function_arg_boundary (replacing & with &&) looks like an oversight of a previous commit in this area which changed 'abi_break' from a boolean to an integer. We also take the opportunity to fix the comment above aarch64_function_arg_alignment since the value of the abi_break parameter was changed in a previous commit, no longer matching the description. 2022-11-28 Christophe Lyon <christophe.lyon@arm.com> Richard Sandiford <richard.sandiford@arm.com> gcc/ChangeLog: * config/aarch64/aarch64.cc (aarch64_function_arg_alignment): Fix comment. (aarch64_layout_arg): Factorize warning conditions. (aarch64_function_arg_boundary): Fix typo. * function.cc (currently_expanding_function_start): New variable. (expand_function_start): Handle currently_expanding_function_start. * function.h (currently_expanding_function_start): Declare. gcc/testsuite/ChangeLog: * gcc.target/aarch64/bitfield-abi-warning-align16-O2.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align32-O2.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align8-O2.c: New test. * gcc.target/aarch64/bitfield-abi-warning.h: New test. * g++.target/aarch64/bitfield-abi-warning-align16-O2.C: New test. * g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C: New test. * g++.target/aarch64/bitfield-abi-warning-align32-O2.C: New test. * g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C: New test. * g++.target/aarch64/bitfield-abi-warning-align8-O2.C: New test. * g++.target/aarch64/bitfield-abi-warning.h: New test.
ecatmur
pushed a commit
that referenced
this pull request
Feb 4, 2023
Here the ahead-of-time overload set pruning in finish_call_expr is unintentionally returning a CALL_EXPR whose (pruned) callee is wrapped in an ADDR_EXPR, despite the original callee not being wrapped in an ADDR_EXPR. This ends up causing a bogus declaration mismatch error in the below testcase because the call to min in #1 gets expressed as a CALL_EXPR of ADDR_EXPR of FUNCTION_DECL, whereas the level-lowered call to min in #2 gets expressed instead as a CALL_EXPR of FUNCTION_DECL. This patch fixes this by stripping the spurious ADDR_EXPR appropriately. Thus the first call to min now also gets expressed as a CALL_EXPR of FUNCTION_DECL, matching the behavior before r12-6075-g2decd2cabe5a4f. PR c++/107461 gcc/cp/ChangeLog: * semantics.cc (finish_call_expr): Strip ADDR_EXPR from the selected callee during overload set pruning. gcc/testsuite/ChangeLog: * g++.dg/template/call9.C: New test.
ecatmur
pushed a commit
that referenced
this pull request
May 17, 2023
… in asm in different mode
See gcc.c-torture/execute/20030222-1.c. Consider the code for 32-bit (e.g. BE) target:
int i, v; long x; x = v; asm ("" : "=r" (i) : "0" (x));
We generate the following RTL with reload insns:
1. subreg:si(x:di, 0) = 0;
2. subreg:si(x:di, 4) = v:si;
3. t:di = x:di, dead x;
4. asm ("" : "=r" (subreg:si(t:di,4)) : "0" (t:di))
5. i:si = subreg:si(t:di,4);
If we assign hard reg of x to t, dead code elimination will remove insn #2
and we will use unitialized hard reg. So exclude the hard reg of x for t.
We could ignore this problem for non-empty asm using all x value but it is hard to
check that the asm are expanded into insn realy using x and setting r.
The old reload pass used the same approach.
gcc/ChangeLog
* lra-constraints.cc (match_reload): Exclude some hard regs for
multi-reg inout reload pseudos used in asm in different mode.
ecatmur
pushed a commit
that referenced
this pull request
May 17, 2023
Currently on xstormy16 SImode shifts by a single bit require two
instructions, and shifts by other non-zero integer immediate constants
require five instructions. This patch implements the obvious optimization
that shifts by two bits can be done in four instructions, by using two
single-bit sequences.
Hence, ashift_2 was previously generated as:
mov r7,r2 | shl r2,#2 | shl r3,#2 | shr r7,gcc-mirror#14 | or r3,r7
ret
and with this patch we now generate:
shl r2,#1 | rlc r3,#1 | shl r2,#1 | rlc r3,#1
ret
2023-04-23 Roger Sayle <roger@nextmovesoftware.com>
gcc/ChangeLog
* config/stormy16/stormy16.cc (xstormy16_output_shift): Implement
SImode shifts by two by performing a single bit SImode shift twice.
gcc/testsuite/ChangeLog
* gcc.target/xstormy16/shiftsi.c: New test case.
ecatmur
pushed a commit
that referenced
this pull request
May 17, 2023
I noticed that for member class templates of a class template we were
unnecessarily substituting both the template and its type. Avoiding that
duplication speeds compilation of this silly testcase from ~12s to ~9s on my
laptop. It's unlikely to make a difference on any real code, but the
simplification is also nice.
We still need to clear CLASSTYPE_USE_TEMPLATE on the partial instantiation
of the template class, but it makes more sense to do that in
tsubst_template_decl anyway.
#define NC(X) \
template <class U> struct X##1; \
template <class U> struct X##2; \
template <class U> struct X##3; \
template <class U> struct X##4; \
template <class U> struct X##5; \
template <class U> struct X##6;
#define NC2(X) NC(X##a) NC(X##b) NC(X##c) NC(X##d) NC(X##e) NC(X##f)
#define NC3(X) NC2(X##A) NC2(X##B) NC2(X##C) NC2(X##D) NC2(X##E)
template <int I> struct A
{
NC3(am)
};
template <class...Ts> void sink(Ts...);
template <int...Is> void g()
{
sink(A<Is>()...);
}
template <int I> void f()
{
g<__integer_pack(I)...>();
}
int main()
{
f<1000>();
}
gcc/cp/ChangeLog:
* pt.cc (instantiate_class_template): Skip the RECORD_TYPE
of a class template.
(tsubst_template_decl): Clear CLASSTYPE_USE_TEMPLATE.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
'void value not ignored as it ought to be' is a holdover from K&R C and is utterly incomprehensible to the modern C++ developer. Switch to 'invalid use of void expression' (already in use in cp/call.cc) and add context/expected type information where available.