[C++ Patch] PR 94034 ("[10 Regression] Broken diagnostic: 'result_decl' not supported by dump_expr")
Hi, in this apparently simple regression we end up producing garbled diagnostic because dump_expr (called via verify_constant) doesn't handle RESULT_DECL. I think it makes sense to add the tree code together with all the other *_DECL. Tested x86_64-linux. Thanks, Paolo. /
Re: [C++ Patch] PR 94034 ("[10 Regression] Broken diagnostic: 'result_decl' not supported by dump_expr")
... the patch ;)
Paolo.
Fix "PR c++/94034 Broken diagnostic: 'result_decl' not supported by dump_expr"
A rather simple diagnostic issue where we failed to handle RESULT_DECL in
dump_expr.
Tested x86_64-linux.
/cp
PR c++/94034
* error.c (dump_expr): Handle RESULT_DECL like the other *_DECL.
/testsuite
PR c++/94034
* g++.dg/cpp0x/pr94034.C: New.
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index cc51b6ffe13..c1392bcbb25 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -2102,6 +2102,7 @@ dump_expr (cxx_pretty_printer *pp, tree t, int flags)
case OVERLOAD:
case TYPE_DECL:
case IDENTIFIER_NODE:
+case RESULT_DECL:
dump_decl (pp, t, ((flags & ~(TFF_DECL_SPECIFIERS|TFF_RETURN_TYPE
|TFF_TEMPLATE_HEADER))
| TFF_NO_TEMPLATE_BINDINGS
diff --git a/gcc/testsuite/g++.dg/cpp0x/pr94034.C
b/gcc/testsuite/g++.dg/cpp0x/pr94034.C
new file mode 100644
index 000..0a828c1e263
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/pr94034.C
@@ -0,0 +1,10 @@
+// { dg-do compile { target c++11 } }
+
+struct A { A *ap = this; };
+constexpr A foo() {
+ return {};
+}
+void g() {
+ constexpr A a = foo(); // { dg-bogus "not supported by" }
+ // { dg-error "not a constant expression" "" { target c++11 } .-1 }
+}
Re: [C++ Patch] PR 90915 [9/10 Regression] ICE in has_attribute, at c-family/c-attribs.c:4221
Hi, On 29/01/20 19:00, Jason Merrill wrote: On 1/29/20 4:31 AM, Paolo Carlini wrote: Hi, in this regression we issue a diagnostic about an incomplete type (only a warning by default) and then we crash when we try to query has_attribute on a member of the type because in such cases the member remains an IDENTIFIER_NODE which of course doesn't have a TREE_TYPE neither a DECL_ATTRIBUTES... Simply recognizing IDENTIFIER_NODEs and returning false works fine, not sure if we want to do something more sophisticated. Tested x86_64-linux. Why are we getting to has_attribute at all for a type-dependent argument? Because the implementation of __builtin_has_attribute, largely shared with the C front-end, doesn't know about templates at all? :-/ Not sure it's the best time to complete it, but shouldn't be too difficult. Paolo.
[C++ Patch] PR 90915 [9/10 Regression] ICE in has_attribute, at c-family/c-attribs.c:4221
Hi,
in this regression we issue a diagnostic about an incomplete type (only
a warning by default) and then we crash when we try to query
has_attribute on a member of the type because in such cases the member
remains an IDENTIFIER_NODE which of course doesn't have a TREE_TYPE
neither a DECL_ATTRIBUTES... Simply recognizing IDENTIFIER_NODEs and
returning false works fine, not sure if we want to do something more
sophisticated. Tested x86_64-linux.
Thanks, Paolo.
Fix "PR c++/90915 ICE in has_attribute, at c-family/c-attribs.c:4221"
A rather simple ICE where we didn't properly recover upon diagnosing
an incomplete type.
Tested x86_64-linux.
/c-family
PR c++/90915
* c-attribs.c (has_attribute): Handle correctly IDENTIFIER_NODEs.
/testsuite
PR c++/90915
* g++.dg/ext/builtin-has-attribute-1.C: New.
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index dc9579c5c60..97590a19c0f 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -4759,6 +4759,10 @@ has_attribute (location_t atloc, tree t, tree attr, tree
(*convert)(tree))
expr = NULL_TREE;
done = true;
}
+ else if (TREE_CODE (expr) == IDENTIFIER_NODE)
+ /* Can happen during error-recovery when querying a member of
+ an incomplete type (c++/90915). */
+ return false;
else if (DECL_P (expr))
{
/* Set TYPE to the DECL's type to process it on the next
diff --git a/gcc/testsuite/g++.dg/ext/builtin-has-attribute-1.C
b/gcc/testsuite/g++.dg/ext/builtin-has-attribute-1.C
new file mode 100644
index 000..3d81a078159
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/builtin-has-attribute-1.C
@@ -0,0 +1,7 @@
+// { dg-do compile { target c++11 } }
+
+struct S;
+template struct T
+{
+ static_assert (!__builtin_has_attribute (((S *) 0) -> a, packed), ""); // {
dg-error "invalid use of incomplete type" }
+};
Re: [C++ Patch] PR 92804 [10 Regression] ICE trying to use concept as a nested-name-specifier
Hi,
On 22/01/20 22:32, Jason Merrill wrote:
On 1/22/20 3:31 PM, Paolo Carlini wrote:
Hi,
On 22/01/20 17:27, Jason Merrill wrote:
On 1/22/20 10:22 AM, Paolo Carlini wrote:
Hi,
in this simple issue we either wrongly talked about variable
template-id in c++17 mode or ICEd in c++2a. I think we simply want
to handle concept-ids first, both as represented in c++17 mode and
as represented in c++2a mode. Tested x86_64-linux.
What happens if you try to use a function template/function concept
name?
AFAICS no ICEs, no regressions but indeed we can do better, tell
concepts from function templates. The below does that and passes
testing but I'm not sure if the wording is optimal, whether we always
want to talk about concept-id.
I think it's fine either way.
+// { dg-do compile { target c++17 } }
+// { dg-options "-w -fconcepts" }
Does it work to use -fconcepts-ts instead of -w -fconcepts?
Sure, it does. Attached what I tested.
Thanks, Paolo.
//
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index caafbefda8e..85a4ea5be82 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6467,16 +6467,27 @@ cp_parser_nested_name_specifier_opt (cp_parser *parser,
tree fns = get_fns (tid);
if (OVL_SINGLE_P (fns))
tmpl = OVL_FIRST (fns);
- error_at (token->location, "function template-id %qD "
- "in nested-name-specifier", tid);
+ if (function_concept_p (fns))
+ error_at (token->location, "concept-id %qD "
+ "in nested-name-specifier", tid);
+ else
+ error_at (token->location, "function template-id "
+ "%qD in nested-name-specifier", tid);
}
else
{
- /* Variable template. */
tmpl = TREE_OPERAND (tid, 0);
- gcc_assert (variable_template_p (tmpl));
- error_at (token->location, "variable template-id %qD "
- "in nested-name-specifier", tid);
+ if (variable_concept_p (tmpl)
+ || standard_concept_p (tmpl))
+ error_at (token->location, "concept-id %qD "
+ "in nested-name-specifier", tid);
+ else
+ {
+ /* Variable template. */
+ gcc_assert (variable_template_p (tmpl));
+ error_at (token->location, "variable template-id "
+ "%qD in nested-name-specifier", tid);
+ }
}
if (tmpl)
inform (DECL_SOURCE_LOCATION (tmpl),
diff --git a/gcc/testsuite/g++.dg/concepts/pr92804-1.C
b/gcc/testsuite/g++.dg/concepts/pr92804-1.C
new file mode 100644
index 000..cc21426bb9e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr92804-1.C
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-fconcepts" }
+
+template
+concept foo = true; // { dg-message "declared here" }
+
+template
+void bar(T t)
+{
+ if constexpr (foo::value) // { dg-error "17:concept-id .foo. in
nested-name-specifier" }
+ // { dg-error "expected|value" "" { target c++17 } .-1 }
+ {
+ }
+}
+
+int main()
+{
+ bar(1);
+}
diff --git a/gcc/testsuite/g++.dg/concepts/pr92804-2.C
b/gcc/testsuite/g++.dg/concepts/pr92804-2.C
new file mode 100644
index 000..32a15543310
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr92804-2.C
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-fconcepts-ts" }
+
+template
+concept bool foo() { return true; }; // { dg-message "declared here" }
+
+template
+void bar(T t)
+{
+ if constexpr (foo::value) // { dg-error "17:concept-id .foo. in
nested-name-specifier" }
+ // { dg-error "expected|value" "" { target *-*-* } .-1 }
+ {
+ }
+}
+
+int main()
+{
+ bar(1);
+}
Re: [C++ Patch] PR 92804 [10 Regression] ICE trying to use concept as a nested-name-specifier
Hi,
On 22/01/20 17:27, Jason Merrill wrote:
On 1/22/20 10:22 AM, Paolo Carlini wrote:
Hi,
in this simple issue we either wrongly talked about variable
template-id in c++17 mode or ICEd in c++2a. I think we simply want to
handle concept-ids first, both as represented in c++17 mode and as
represented in c++2a mode. Tested x86_64-linux.
What happens if you try to use a function template/function concept name?
AFAICS no ICEs, no regressions but indeed we can do better, tell
concepts from function templates. The below does that and passes testing
but I'm not sure if the wording is optimal, whether we always want to
talk about concept-id.
Thanks! Paolo.
///
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index caafbefda8e..85a4ea5be82 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6467,16 +6467,27 @@ cp_parser_nested_name_specifier_opt (cp_parser *parser,
tree fns = get_fns (tid);
if (OVL_SINGLE_P (fns))
tmpl = OVL_FIRST (fns);
- error_at (token->location, "function template-id %qD "
- "in nested-name-specifier", tid);
+ if (function_concept_p (fns))
+ error_at (token->location, "concept-id %qD "
+ "in nested-name-specifier", tid);
+ else
+ error_at (token->location, "function template-id "
+ "%qD in nested-name-specifier", tid);
}
else
{
- /* Variable template. */
tmpl = TREE_OPERAND (tid, 0);
- gcc_assert (variable_template_p (tmpl));
- error_at (token->location, "variable template-id %qD "
- "in nested-name-specifier", tid);
+ if (variable_concept_p (tmpl)
+ || standard_concept_p (tmpl))
+ error_at (token->location, "concept-id %qD "
+ "in nested-name-specifier", tid);
+ else
+ {
+ /* Variable template. */
+ gcc_assert (variable_template_p (tmpl));
+ error_at (token->location, "variable template-id "
+ "%qD in nested-name-specifier", tid);
+ }
}
if (tmpl)
inform (DECL_SOURCE_LOCATION (tmpl),
diff --git a/gcc/testsuite/g++.dg/concepts/pr92804-1.C
b/gcc/testsuite/g++.dg/concepts/pr92804-1.C
new file mode 100644
index 000..cc21426bb9e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr92804-1.C
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-fconcepts" }
+
+template
+concept foo = true; // { dg-message "declared here" }
+
+template
+void bar(T t)
+{
+ if constexpr (foo::value) // { dg-error "17:concept-id .foo. in
nested-name-specifier" }
+ // { dg-error "expected|value" "" { target c++17 } .-1 }
+ {
+ }
+}
+
+int main()
+{
+ bar(1);
+}
diff --git a/gcc/testsuite/g++.dg/concepts/pr92804-2.C
b/gcc/testsuite/g++.dg/concepts/pr92804-2.C
new file mode 100644
index 000..98ec4ae598e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr92804-2.C
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-w -fconcepts" }
+
+template
+concept bool foo() { return true; }; // { dg-message "declared here" }
+
+template
+void bar(T t)
+{
+ if constexpr (foo::value) // { dg-error "17:concept-id .foo. in
nested-name-specifier" }
+ // { dg-error "expected|value" "" { target *-*-* } .-1 }
+ {
+ }
+}
+
+int main()
+{
+ bar(1);
+}
[C++ Patch] PR 92804 [10 Regression] ICE trying to use concept as a nested-name-specifier
Hi,
in this simple issue we either wrongly talked about variable template-id
in c++17 mode or ICEd in c++2a. I think we simply want to handle
concept-ids first, both as represented in c++17 mode and as represented
in c++2a mode. Tested x86_64-linux.
Thanks, Paolo.
///
Fix "PR c++/92804 ICE trying to use concept as a nested-name-specifier"
A rather simple ICE where we failed to properly check for concept-ids
uses in nested-name-specifiers.
Tested x86_64-linux.
/cp
PR c++/92804
* parser.c (cp_parser_nested_name_specifier_opt): Properly
diagnose concept-ids.
/testsuite
PR c++/92804
* g++.dg/concepts/pr92804.C: New.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index caafbefda8e..fe490d4a69f 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6472,11 +6472,18 @@ cp_parser_nested_name_specifier_opt (cp_parser *parser,
}
else
{
- /* Variable template. */
tmpl = TREE_OPERAND (tid, 0);
- gcc_assert (variable_template_p (tmpl));
- error_at (token->location, "variable template-id %qD "
- "in nested-name-specifier", tid);
+ if (variable_concept_p (tmpl)
+ || standard_concept_p (tmpl))
+ error_at (token->location, "concept-id %qD "
+ "in nested-name-specifier", tid);
+ else
+ {
+ /* Variable template. */
+ gcc_assert (variable_template_p (tmpl));
+ error_at (token->location, "variable template-id "
+ "%qD in nested-name-specifier", tid);
+ }
}
if (tmpl)
inform (DECL_SOURCE_LOCATION (tmpl),
diff --git a/gcc/testsuite/g++.dg/concepts/pr92804.C
b/gcc/testsuite/g++.dg/concepts/pr92804.C
new file mode 100644
index 000..cc21426bb9e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr92804.C
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-fconcepts" }
+
+template
+concept foo = true; // { dg-message "declared here" }
+
+template
+void bar(T t)
+{
+ if constexpr (foo::value) // { dg-error "17:concept-id .foo. in
nested-name-specifier" }
+ // { dg-error "expected|value" "" { target c++17 } .-1 }
+ {
+ }
+}
+
+int main()
+{
+ bar(1);
+}
Re: [C++ Patch] Improve build_new locations
Hi,
On 06/01/20 21:47, Jason Merrill wrote:
On 1/2/20 4:23 AM, Paolo Carlini wrote:
@@ -19320,8 +19320,8 @@ tsubst_copy_and_build (tree t,
tree op1 = tsubst (TREE_OPERAND (t, 1), args, complain,
in_decl);
tree op2 = RECUR (TREE_OPERAND (t, 2));
- ret = build_new (_vec, op1, op2, _vec,
- NEW_EXPR_USE_GLOBAL (t),
+ ret = build_new (input_location, _vec, op1, op2,
Hmm, using input_location bothers me even though it does have the
right value in this function. Let's use a local variable instead;
maybe change the existing loc variable to save_loc and use the name
loc for the location we want to use.
I see, I wondered about that myself: when I started working on these
location issues I forwarded simple EXPR_LOCATION (t) in a few places,
which worked just fine. Then I noticed that in many existing places we
were exploiting the fact that input_location is changed on top and
started using it myself too. I suppose some of those places too could be
changed to simple EXPR_LOCATION (t), I can experiment with that.
Anyway, the below implements the save_loc bit and regtests fine on
x86_64-linux, as usual.
Thanks, Paolo.
/
Index: gcc/cp/cp-tree.h
===
--- gcc/cp/cp-tree.h(revision 279829)
+++ gcc/cp/cp-tree.h(working copy)
@@ -6720,9 +6720,10 @@ extern tree throw_bad_array_new_length (void);
extern bool type_has_new_extended_alignment(tree);
extern unsigned malloc_alignment (void);
extern tree build_new_constexpr_heap_type (tree, tree, tree);
-extern tree build_new (vec **, tree,
tree,
-vec **, int,
- tsubst_flags_t);
+extern tree build_new (location_t,
+vec **, tree,
+tree, vec **,
+int, tsubst_flags_t);
extern tree get_temp_regvar(tree, tree);
extern tree build_vec_init (tree, tree, tree, bool, int,
tsubst_flags_t);
Index: gcc/cp/init.c
===
--- gcc/cp/init.c (revision 279829)
+++ gcc/cp/init.c (working copy)
@@ -2396,8 +2396,8 @@ decl_constant_value (tree decl)
creates and returns a NEW_EXPR. */
static tree
-build_raw_new_expr (vec *placement, tree type, tree nelts,
- vec *init, int use_global_new)
+build_raw_new_expr (location_t loc, vec *placement, tree type,
+ tree nelts, vec *init, int use_global_new)
{
tree init_list;
tree new_expr;
@@ -2413,9 +2413,9 @@ static tree
else
init_list = build_tree_list_vec (init);
- new_expr = build4 (NEW_EXPR, build_pointer_type (type),
-build_tree_list_vec (placement), type, nelts,
-init_list);
+ new_expr = build4_loc (loc, NEW_EXPR, build_pointer_type (type),
+build_tree_list_vec (placement), type, nelts,
+init_list);
NEW_EXPR_USE_GLOBAL (new_expr) = use_global_new;
TREE_SIDE_EFFECTS (new_expr) = 1;
@@ -3775,8 +3775,9 @@ build_new_1 (vec **placement, tree ty
rather than just "new". This may change PLACEMENT and INIT. */
tree
-build_new (vec **placement, tree type, tree nelts,
- vec **init, int use_global_new, tsubst_flags_t complain)
+build_new (location_t loc, vec **placement, tree type,
+ tree nelts, vec **init, int use_global_new,
+ tsubst_flags_t complain)
{
tree rval;
vec *orig_placement = NULL;
@@ -3826,7 +3827,7 @@ tree
|| (nelts && type_dependent_expression_p (nelts))
|| (nelts && *init)
|| any_type_dependent_arguments_p (*init))
- return build_raw_new_expr (*placement, type, nelts, *init,
+ return build_raw_new_expr (loc, *placement, type, nelts, *init,
use_global_new);
orig_placement = make_tree_vector_copy (*placement);
@@ -3852,10 +3853,11 @@ tree
if (nelts)
{
+ location_t nelts_loc = cp_expr_loc_or_loc (nelts, loc);
if (!build_expr_type_conversion (WANT_INT | WANT_ENUM, nelts, false))
{
if (complain & tf_error)
- permerror (cp_expr_loc_or_input_loc (nelts),
+ permerror (nelts_loc,
"size in array new must have integral type");
else
return error_mark_node;
@@ -3871,8 +3873,7 @@ tree
less than zero. ... If the expression is a constant expression,
the program is ill-fomed. */
if (TREE_CODE (cst_nelts) == INTEGER_CST
- && !val
[C++ Patch] Improve build_new locations
Hi,
some rather straightforward bits for build_new /
cp_parser_new_expression, almost a clean-up. It would be nice to do
better for the diagnostic issued from build_new_1 too but having
consistent locations for all the valid_array_size_p and
invalid_array_size_error calls isn't trivial, I'd rather work on that at
a later time. Tested x86_64-linux.
Thanks, Paolo.
///
/gcc/cp
2020-01-02 Paolo Carlini
* init.c (build_new): Add location_t parameter and use it throughout.
(build_raw_new_expr): Likewise.
* parser.c (cp_parser_new_expression): Pass the combined_loc.
* pt.c (tsubst_copy_and_build): Adjust call.
* cp-tree.h: Update declarations.
/libcc1
2020-01-02 Paolo Carlini
* libcp1plugin.cc (plugin_build_new_expr): Update build_new call.
/gcc/testsuite
2020-01-02 Paolo Carlini
* g++.old-deja/g++.bugs/900208_03.C: Check locations too.
* g++.old-deja/g++.bugs/900519_06.C: Likewise.
Index: gcc/cp/cp-tree.h
===
--- gcc/cp/cp-tree.h(revision 279768)
+++ gcc/cp/cp-tree.h(working copy)
@@ -6720,9 +6720,10 @@ extern tree throw_bad_array_new_length (void);
extern bool type_has_new_extended_alignment(tree);
extern unsigned malloc_alignment (void);
extern tree build_new_constexpr_heap_type (tree, tree, tree);
-extern tree build_new (vec **, tree,
tree,
-vec **, int,
- tsubst_flags_t);
+extern tree build_new (location_t,
+vec **, tree,
+tree, vec **,
+int, tsubst_flags_t);
extern tree get_temp_regvar(tree, tree);
extern tree build_vec_init (tree, tree, tree, bool, int,
tsubst_flags_t);
Index: gcc/cp/init.c
===
--- gcc/cp/init.c (revision 279768)
+++ gcc/cp/init.c (working copy)
@@ -2396,8 +2396,8 @@ decl_constant_value (tree decl)
creates and returns a NEW_EXPR. */
static tree
-build_raw_new_expr (vec *placement, tree type, tree nelts,
- vec *init, int use_global_new)
+build_raw_new_expr (location_t loc, vec *placement, tree type,
+ tree nelts, vec *init, int use_global_new)
{
tree init_list;
tree new_expr;
@@ -2413,9 +2413,9 @@ static tree
else
init_list = build_tree_list_vec (init);
- new_expr = build4 (NEW_EXPR, build_pointer_type (type),
-build_tree_list_vec (placement), type, nelts,
-init_list);
+ new_expr = build4_loc (loc, NEW_EXPR, build_pointer_type (type),
+build_tree_list_vec (placement), type, nelts,
+init_list);
NEW_EXPR_USE_GLOBAL (new_expr) = use_global_new;
TREE_SIDE_EFFECTS (new_expr) = 1;
@@ -3775,8 +3775,9 @@ build_new_1 (vec **placement, tree ty
rather than just "new". This may change PLACEMENT and INIT. */
tree
-build_new (vec **placement, tree type, tree nelts,
- vec **init, int use_global_new, tsubst_flags_t complain)
+build_new (location_t loc, vec **placement, tree type,
+ tree nelts, vec **init, int use_global_new,
+ tsubst_flags_t complain)
{
tree rval;
vec *orig_placement = NULL;
@@ -3826,7 +3827,7 @@ tree
|| (nelts && type_dependent_expression_p (nelts))
|| (nelts && *init)
|| any_type_dependent_arguments_p (*init))
- return build_raw_new_expr (*placement, type, nelts, *init,
+ return build_raw_new_expr (loc, *placement, type, nelts, *init,
use_global_new);
orig_placement = make_tree_vector_copy (*placement);
@@ -3852,10 +3853,11 @@ tree
if (nelts)
{
+ location_t nelts_loc = cp_expr_loc_or_loc (nelts, loc);
if (!build_expr_type_conversion (WANT_INT | WANT_ENUM, nelts, false))
{
if (complain & tf_error)
- permerror (cp_expr_loc_or_input_loc (nelts),
+ permerror (nelts_loc,
"size in array new must have integral type");
else
return error_mark_node;
@@ -3871,8 +3873,7 @@ tree
less than zero. ... If the expression is a constant expression,
the program is ill-fomed. */
if (TREE_CODE (cst_nelts) == INTEGER_CST
- && !valid_array_size_p (cp_expr_loc_or_input_loc (nelts),
- cst_nelts, NULL_TREE,
+ && !valid_array_size_p (nelts_loc, cst_nelts, NULL_TREE,
complain & tf_error))
[C++ Patch] Improve delete expressions locations
Hi,
more of the last idea, this time applied to cp_parser_delete_expression
/ delete_sanity (thus build_delete and build_vec_delete which provide
tests). Nothing particularly remarkable in this case. Tested x86_64-linux.
Thanks, Paolo.
///
/gcc/cp
2019-12-20 Paolo Carlini
* decl2.c (delete_sanity): Add location_t parameter and use
it throughout.
* init.c (build_vec_delete_1): Likewise.
(build_delete): Likewise.
(build_vec_delete): Likewise.
(perform_target_ctor): Adjust call.
(perform_member_init): Likewise.
(build_vec_init): Likewise.
* decl.c (cxx_maybe_build_cleanup): Likewise.
* pt.c (tsubst_copy_and_build): Likewise.
* parser.c (cp_parser_delete_expression): Likewise, pass the
combined_loc.
* cp-tree.h: Update declarations.
/libcc1
2019-12-20 Paolo Carlini
* libcp1plugin.cc (plugin_build_unary_expr): Update delete_sanity
call.
/gcc/testsuite
2019-12-18 Paolo Carlini
* g++.dg/init/delete1.C: Check locations too.
* g++.dg/ipa/pr85607.C: Likewise.
* g++.dg/warn/Wdelete-incomplete-1.C: Likewise.
* g++.dg/warn/delete-non-virtual-dtor.C: Likewise.
* g++.dg/warn/incomplete1.C: Likewise.
Index: gcc/cp/cp-tree.h
===
--- gcc/cp/cp-tree.h(revision 279563)
+++ gcc/cp/cp-tree.h(working copy)
@@ -6583,7 +6583,8 @@ extern bool vague_linkage_p (tree);
extern void grokclassfn(tree, tree,
enum overload_flags);
extern tree grok_array_decl(location_t, tree, tree, bool);
-extern tree delete_sanity (tree, tree, bool, int,
tsubst_flags_t);
+extern tree delete_sanity (location_t, tree, tree, bool,
+int, tsubst_flags_t);
extern tree check_classfn (tree, tree, tree);
extern void check_member_template (tree);
extern tree grokfield (const cp_declarator *, cp_decl_specifier_seq *,
@@ -6725,11 +6726,11 @@ extern tree build_new
(vec **, tre
extern tree get_temp_regvar(tree, tree);
extern tree build_vec_init (tree, tree, tree, bool, int,
tsubst_flags_t);
-extern tree build_delete (tree, tree,
+extern tree build_delete (location_t, tree, tree,
special_function_kind,
int, int, tsubst_flags_t);
extern void push_base_cleanups (void);
-extern tree build_vec_delete (tree, tree,
+extern tree build_vec_delete (location_t, tree, tree,
special_function_kind, int,
tsubst_flags_t);
extern tree create_temporary_var (tree);
Index: gcc/cp/decl.c
===
--- gcc/cp/decl.c (revision 279563)
+++ gcc/cp/decl.c (working copy)
@@ -17311,7 +17311,7 @@ cxx_maybe_build_cleanup (tree decl, tsubst_flags_t
else
addr = build_address (decl);
- call = build_delete (TREE_TYPE (addr), addr,
+ call = build_delete (input_location, TREE_TYPE (addr), addr,
sfk_complete_destructor, flags, 0, complain);
if (call == error_mark_node)
cleanup = error_mark_node;
Index: gcc/cp/decl2.c
===
--- gcc/cp/decl2.c (revision 279563)
+++ gcc/cp/decl2.c (working copy)
@@ -475,8 +475,8 @@ grok_array_decl (location_t loc, tree array_expr,
Implements ARM $5.3.4. This is called from the parser. */
tree
-delete_sanity (tree exp, tree size, bool doing_vec, int use_global_delete,
- tsubst_flags_t complain)
+delete_sanity (location_t loc, tree exp, tree size, bool doing_vec,
+ int use_global_delete, tsubst_flags_t complain)
{
tree t, type;
@@ -489,14 +489,16 @@ tree
DELETE_EXPR_USE_GLOBAL (t) = use_global_delete;
DELETE_EXPR_USE_VEC (t) = doing_vec;
TREE_SIDE_EFFECTS (t) = 1;
+ SET_EXPR_LOCATION (t, loc);
return t;
}
+ location_t exp_loc = cp_expr_loc_or_loc (exp, loc);
+
/* An array can't have been allocated by new, so complain. */
if (TREE_CODE (TREE_TYPE (exp)) == ARRAY_TYPE
&& (complain & tf_warning))
-warning_at (cp_expr_loc_or_input_loc (exp), 0,
- "deleting array %q#E", exp);
+warning_at (exp_loc, 0, "deleting array %q#E", exp);
t = build_expr_type_conversion (WANT_POINTER
[C++ Patch] Improve throw, sizeof, and alignof locations & more
Hi, another batch of work. Primarily, more of the idea of moving up the construction of the compound location thus passing it to the cxx_sizeof_or_alignof* and build_throw functions to obtain better locations for all the diagnostics issued by the latter. During the work a few mildly interesting nits (eg, for sizeof and alignof we want to set whenever possible the location inside the cxx_* functions and that allows to avoid set_location in cp_parser_unary_expression, but minimally we have to pass it to the cp_expr constructor, otherwise plugin testcases badly fail; noticed that in cxx_sizeof_expr and cxx_alignof_expr having a single cp_expr_loc_or_input_loc on top actually implies better locations (4 testcases) because below, before the diagnostic calls, we have STRIP_ANY_LOCATION_WRAPPER uses; in fact I changed those cp_expr_loc_or_input_loc to cp_expr_loc_or_loc because for expressions we still want to try fetching the locations but we do have a meaningful fallback in the location argument of the function; in build_throw, an error should be an inform, because in such cases we already issued an error (tested in ctor1.C)) but nothing major. The below includes a few other minor changes, like two additional uses of DECL_SOURCE_LOCATION, cp_expr_loc_or_input_loc, removal of an unused function. Tested x86_64-linux. Thanks, Paolo. // /gcc/cp 2019-12-16 Paolo Carlini * typeck.c (cxx_sizeof_or_alignof_type): Add location_t parameter and use it throughout. (cxx_sizeof_expr): Likewise. (cxx_alignof_expr): Likewise. (cxx_sizeof_or_alignof_expr): Likewise. (cxx_alignas_expr): Update call. * decl.c (fold_sizeof_expr): Likewise. * pt.c (tsubst_copy): Likewise. (tsubst_copy_and_build): Likewise. * except.c (build_throw): Add location_t parameter and use it. (expand_end_catch_block): Update call. * parser.c (cp_parser_unary_expression): Update cxx_sizeof_or_alignof_type and cxx_sizeof_or_alignof_expr calls, pass the compound location. (cp_parser_throw_expression): Likewise pass the combined location to build_throw. * cp-tree.h: Update declarations. * semantics.c (finish_handler_parms): Use DECL_SOURCE_LOCATION. * decl2.c (check_classfn): Likewise. * except.c (is_admissible_throw_operand_or_catch_parameter): Exploit cp_expr_loc_or_input_loc in one place. * except.c (create_try_catch_expr): Remove, unused. /libcc1 2019-12-16 Paolo Carlini * libcp1plugin.cc (plugin_build_unary_expr): Update build_throw and cxx_sizeof_or_alignof_expr calls. (plugin_build_unary_type_expr): Likewise for cxx_sizeof_or_alignof_type. /gcc/testsuite 2019-12-16 Paolo Carlini * g++.dg/diagnostic/alignof2.C: New. * g++.dg/diagnostic/alignof3.C: Likewise. * g++.dg/diagnostic/incomplete-type-1.C: Likewise. * g++.dg/warn/Wcatch-value-3b.C: Likewise. * g++.dg/cpp0x/alignof3.C: Check location(s) too. * g++.dg/cpp1z/decomp-bitfield1.C: Likewise. * g++.dg/cpp1z/has-unique-obj-representations2.C: Likewise. * g++.dg/expr/sizeof3.C: Likewise. * g++.dg/ext/flexary6.C: Likewise. * g++.dg/ext/vla4.C: Likewise. * g++.dg/template/sizeof11.C: Likewise. * g++.dg/warn/Wcatch-value-1.C: Likewise. * g++.dg/warn/Wcatch-value-2.C: Likewise. * g++.dg/warn/Wcatch-value-3.C: Likewise. * g++.old-deja/g++.brendan/sizeof1.C: Likewise. * g++.old-deja/g++.brendan/sizeof3.C: Likewise. * g++.old-deja/g++.brendan/sizeof4.C: Likewise. * g++.old-deja/g++.eh/ctor1.C: Likewise. * g++.old-deja/g++.jason/ambig1.C: Likewise. * g++.old-deja/g++.other/sizeof4.C: Likewise. Index: gcc/cp/cp-tree.h === --- gcc/cp/cp-tree.h(revision 279371) +++ gcc/cp/cp-tree.h(working copy) @@ -6657,7 +6657,7 @@ extern void init_exception_processing (void); extern tree expand_start_catch_block (tree); extern void expand_end_catch_block (void); extern tree build_exc_ptr (void); -extern tree build_throw(tree); +extern tree build_throw(location_t, tree); extern int nothrow_libfn_p (const_tree); extern void check_handlers (tree); extern tree finish_noexcept_expr (tree, tsubst_flags_t); @@ -6674,7 +6674,6 @@ extern tree begin_eh_spec_block (void); extern void finish_eh_spec_block (tree, tree); extern tree build_eh_type_type (tree); extern tree cp_protect_cleanup_actions (void); -extern tree create_try_catch_expr (tree, tree); extern tree template_parms_to_args
Re: [C++ Patch] Improve build_*_cast locations
Hi,
On 08/12/19 18:51, Jason Merrill wrote:
Hmm, is the change to cp_expr really necessary vs. using
protected_set_expr_location?
Yes, using protected_set_expr_location works fine in this case, I
suppose because we are dealing with expressions anyway plus the cp_expr
constructor from a tree copies the location too. In the below I also
added the thin build_functional_case wrapper, this way consistently all
the build_*_cast functions called by the parser do not use set_location
afterwards. Note, at some point we should also do something about the
build_x_* functions which have been doing that for a while...
Anyway, the below passed testing.
Thanks, Paolo.
Index: gcc/cp/cp-tree.h
===
--- gcc/cp/cp-tree.h(revision 279041)
+++ gcc/cp/cp-tree.h(working copy)
@@ -6998,7 +6998,8 @@ extern tree build_typeid (tree,
tsubst_flags_t);
extern tree get_tinfo_decl (tree);
extern tree get_typeid (tree, tsubst_flags_t);
extern tree build_headof (tree);
-extern tree build_dynamic_cast (tree, tree, tsubst_flags_t);
+extern tree build_dynamic_cast (location_t, tree, tree,
+tsubst_flags_t);
extern void emit_support_tinfos(void);
extern bool emit_tinfo_decl(tree);
@@ -7547,13 +7548,17 @@ extern tree build_x_compound_expr
(location_t, tr
tsubst_flags_t);
extern tree build_compound_expr (location_t, tree, tree);
extern tree cp_build_compound_expr (tree, tree, tsubst_flags_t);
-extern tree build_static_cast (tree, tree, tsubst_flags_t);
-extern tree build_reinterpret_cast (tree, tree, tsubst_flags_t);
-extern tree build_const_cast (tree, tree, tsubst_flags_t);
+extern tree build_static_cast (location_t, tree, tree,
+tsubst_flags_t);
+extern tree build_reinterpret_cast (location_t, tree, tree,
+tsubst_flags_t);
+extern tree build_const_cast (location_t, tree, tree,
+tsubst_flags_t);
extern tree build_c_cast (location_t, tree, tree);
extern cp_expr build_c_cast(location_t loc, tree type,
cp_expr expr);
-extern tree cp_build_c_cast(tree, tree, tsubst_flags_t);
+extern tree cp_build_c_cast(location_t, tree, tree,
+tsubst_flags_t);
extern cp_expr build_x_modify_expr (location_t, tree,
enum tree_code, tree,
tsubst_flags_t);
@@ -7613,7 +7618,8 @@ extern int lvalue_or_else (tree, enum
lvalue_use
extern void check_template_keyword (tree);
extern bool check_raw_literal_operator (const_tree decl);
extern bool check_literal_operator_args(const_tree, bool *,
bool *);
-extern void maybe_warn_about_useless_cast (tree, tree, tsubst_flags_t);
+extern void maybe_warn_about_useless_cast (location_t, tree, tree,
+tsubst_flags_t);
extern tree cp_perform_integral_promotions (tree, tsubst_flags_t);
extern tree finish_left_unary_fold_expr (tree, int);
Index: gcc/cp/decl.c
===
--- gcc/cp/decl.c (revision 279041)
+++ gcc/cp/decl.c (working copy)
@@ -6483,7 +6483,8 @@ reshape_init (tree type, tree init, tsubst_flags_t
{
warning_sentinel w (warn_useless_cast);
warning_sentinel w2 (warn_ignored_qualifiers);
- return cp_build_c_cast (type, elt, tf_warning_or_error);
+ return cp_build_c_cast (input_location, type, elt,
+ tf_warning_or_error);
}
else
return error_mark_node;
Index: gcc/cp/method.c
===
--- gcc/cp/method.c (revision 279041)
+++ gcc/cp/method.c (working copy)
@@ -474,7 +474,8 @@ forward_parm (tree parm)
if (!TYPE_REF_P (type))
type = cp_build_reference_type (type, /*rval=*/true);
warning_sentinel w (warn_useless_cast);
- exp = build_static_cast (type, exp, tf_warning_or_error);
+ exp = build_static_cast (input_location, type, exp,
+ tf_warning_or_error);
if (DECL_PACK_P (parm))
exp = make_pack_expansion (exp);
return exp;
@@ -1361,7 +1362,8 @@ build_comparison_op (tree fndecl,
Re: [C++ Patch] Improve build_*_cast locations
Hi, On 06/12/19 18:53, Jason Merrill wrote: On 12/6/19 11:14 AM, Paolo Carlini wrote: Hi, as promised, the below takes care of the various remaining casts. It's mostly mechanical, follows the same approach used for build_functional_cast. Tested x86_64-linux. It occurs to me that once we're passing the location into the build_* functions, we can apply it to the expression there rather than in the parser. In fact that occurred to me too ;) but at first I didn't like the idea of multiplying the set_location calls... Anyway, in practice for the casts of this last patch the idea works reasonably well, because most of the complexity is encapsulated in the *_1 versions of the functions and the build_* functions proper have only a couple of returns. I wanted to send those changes... But then my build failed in the libcp1plugin.cc bits because the switch also includes c_cast and all the functions must return the same type. And then for consistency we want also to adjust functional_cast (which allows to remove the set_location in the parser which also remained after my previous patch). Those two cast however are more annoying, because there aren't *_1 counterparts and the functions have *lots* of returns, are complicated. Thus for those I'm proposing to create ad hoc *_1 identical to the current functions in order to have single set_location in cp_build_c_cast and build_functional_cast. All in all, I don't have better ideas... I'm finishing testing the below. Thanks, Paolo. // Index: gcc/cp/cp-tree.h === --- gcc/cp/cp-tree.h(revision 279041) +++ gcc/cp/cp-tree.h(working copy) @@ -6998,7 +6998,8 @@ extern tree build_typeid (tree, tsubst_flags_t); extern tree get_tinfo_decl (tree); extern tree get_typeid (tree, tsubst_flags_t); extern tree build_headof (tree); -extern tree build_dynamic_cast (tree, tree, tsubst_flags_t); +extern cp_expr build_dynamic_cast (location_t, tree, tree, +tsubst_flags_t); extern void emit_support_tinfos(void); extern bool emit_tinfo_decl(tree); @@ -7547,13 +7548,17 @@ extern tree build_x_compound_expr (location_t, tr tsubst_flags_t); extern tree build_compound_expr (location_t, tree, tree); extern tree cp_build_compound_expr (tree, tree, tsubst_flags_t); -extern tree build_static_cast (tree, tree, tsubst_flags_t); -extern tree build_reinterpret_cast (tree, tree, tsubst_flags_t); -extern tree build_const_cast (tree, tree, tsubst_flags_t); +extern cp_expr build_static_cast (location_t, tree, tree, +tsubst_flags_t); +extern cp_expr build_reinterpret_cast (location_t, tree, tree, +tsubst_flags_t); +extern cp_expr build_const_cast(location_t, tree, tree, +tsubst_flags_t); extern tree build_c_cast (location_t, tree, tree); extern cp_expr build_c_cast(location_t loc, tree type, cp_expr expr); -extern tree cp_build_c_cast(tree, tree, tsubst_flags_t); +extern cp_expr cp_build_c_cast (location_t, tree, tree, +tsubst_flags_t); extern cp_expr build_x_modify_expr (location_t, tree, enum tree_code, tree, tsubst_flags_t); @@ -7613,7 +7618,8 @@ extern int lvalue_or_else (tree, enum lvalue_use extern void check_template_keyword (tree); extern bool check_raw_literal_operator (const_tree decl); extern bool check_literal_operator_args(const_tree, bool *, bool *); -extern void maybe_warn_about_useless_cast (tree, tree, tsubst_flags_t); +extern void maybe_warn_about_useless_cast (location_t, tree, tree, +tsubst_flags_t); extern tree cp_perform_integral_promotions (tree, tsubst_flags_t); extern tree finish_left_unary_fold_expr (tree, int); @@ -7681,7 +7687,7 @@ extern tree build_scoped_ref (tree, tree, tree * extern tree build_x_arrow (location_t, tree, tsubst_flags_t); extern tree build_m_component_ref (tree, tree, tsubst_flags_t); -extern tree build_functional_cast (location_t, tree, tree, +extern cp_expr build_functional_cast
[C++ Patch] Improve build_*_cast locations
Hi, as promised, the below takes care of the various remaining casts. It's mostly mechanical, follows the same approach used for build_functional_cast. Tested x86_64-linux. Thanks, Paolo. gcc/cp 2019-12-06 Paolo Carlini * typeck.c (check_for_casting_away_constness): Add location_t parameter and use it. (maybe_warn_about_useless_cast): Likewise. (maybe_warn_about_cast_ignoring_quals): Likewise. (build_static_cast_1): Likewise. (build_static_cast): Likewise. (build_reinterpret_cast_1): Likewise. (build_reinterpret_cast): Likewise. (build_const_cast_1): Likewise. (build_const_cast): Likewise. (cp_build_c_cast): Likewise. (build_c_cast): Adjust. (build_ptrmemfunc): Adjust calls. (cp_build_unary_op): Pass the location to invert_truthvalue_loc. * rtti.c (build_dynamic_cast_1): Add location_t parameter and use it. (build_dynamic_cast): Likewise. * cp-tree.h: Adjust declarations. * parser.c (cp_parser_postfix_expression): Pass cp_cast_loc to the various build_*_cast functions. (get_cast_suggestion): Adjust calls. (cp_parser_builtin_offsetof): Likewise. * decl.c (reshape_init): Adjust call. * method.c (forward_parm): Likewise. (build_comparison_op): Likewise. * pt.c (tsubst_copy_and_build): Likewise. * semantics.c (finish_omp_reduction_clause): Likewise. (cp_omp_finish_iterators): Likewise. * tree.c (cp_stabilize_reference): Likewise. (move): Likewise. * typeck2.c (build_functional_cast): Likewise. /libcc1 2019-12-06 Paolo Carlini * libcp1plugin.cc (plugin_build_cast_expr): Adjust build_cast declaration. gcc/testsuite 2019-12-06 Paolo Carlini * c-c++-common/Wcast-align.c: Check location(s) too. * c-c++-common/Wcast-function-type.c: Likewise. * c-c++-common/Wint-to-pointer-cast-1.c: Likewise. * c-c++-common/Wint-to-pointer-cast-2.c: Likewise. * c-c++-common/Wint-to-pointer-cast-3.c: Likewise. * g++.dg/Wcast-function-type.C: Likewise. * g++.dg/addr_builtin-1.C: Likewise. * g++.dg/conversion/const2.C: Likewise. * g++.dg/conversion/dynamic1.C: Likewise. * g++.dg/conversion/ptrmem2.C: Likewise. * g++.dg/conversion/ptrmem3.C: Likewise. * g++.dg/conversion/qual3.C: Likewise. * g++.dg/conversion/reinterpret3.C: Likewise. * g++.dg/cpp0x/lambda/lambda-conv11.C: Likewise. * g++.dg/cpp0x/nullptr04.C: Likewise. * g++.dg/cpp0x/reinterpret_cast2.C: Likewise. * g++.dg/cpp0x/rv-cast2.C: Likewise. * g++.dg/cpp1y/lambda-conv1.C: Likewise. * g++.dg/cpp1z/noexcept-type7.C: Likewise. * g++.dg/cpp2a/array-conv9.C: Likewise. * g++.dg/expr/cast11.C: Likewise. * g++.dg/expr/static_cast8.C: Likewise. * g++.dg/ext/vector6.C: Likewise. * g++.dg/other/conversion1.C: Likewise. * g++.dg/parse/pr26997.C: Likewise. * g++.dg/rtti/no-rtti.C: Likewise. * g++.dg/tc1/dr137.C: Likewise. * g++.dg/template/cast4.C: Likewise. * g++.dg/warn/Wcast-qual1.C: Likewise. * g++.dg/warn/Wcast-qual2.C: Likewise. * g++.dg/warn/Wconditionally-supported-1.C: Likewise. * g++.dg/warn/Wuseless-cast.C: Likewise. * g++.dg/warn/pr35711.C: Likewise. * g++.old-deja/g++.bugs/900227_01.C: Likewise. * g++.old-deja/g++.bugs/900404_07.C: Likewise. * g++.old-deja/g++.jason/overload1.C: Likewise. * g++.old-deja/g++.jason/rfg26.C: Likewise. * g++.old-deja/g++.jason/rvalue3.C: Likewise. * g++.old-deja/g++.jason/warning2.C: Likewise. * g++.old-deja/g++.mike/dyncast4.C: Likewise. * g++.old-deja/g++.mike/dyncast6.C: Likewise. * g++.old-deja/g++.mike/p11482.C: Likewise. * g++.old-deja/g++.mike/p2573.C: Likewise. * g++.old-deja/g++.mike/p2855.C: Likewise. * g++.old-deja/g++.mike/p7476.C: Likewise. * g++.old-deja/g++.mike/p8039.C: Likewise. * g++.old-deja/g++.other/cast2.C: Likewise. * g++.old-deja/g++.other/cast3.C: Likewise. * g++.old-deja/g++.other/dcast1.C: Likewise. * g++.old-deja/g++.other/dcast2.C: Likewise. Index: gcc/cp/cp-tree.h === --- gcc/cp/cp-tree.h(revision 279041) +++ gcc/cp/cp-tree.h(working copy) @@ -6998,7 +6998,8 @@ extern tree build_typeid (tree, tsubst_flags_t); extern tree get_tinfo_decl (tree); extern tree get_typeid (tree, tsubst_flags_t); extern tree build_headof (tree); -extern tree build_dynamic_cast (tree, tree, tsubst_flags_t); +extern tree build_dynamic_cast (location_t, tree
[C++ Patch] Improve build_functional_cast locations
Hi,
this exemplifies in a compact way the kind of change we can implement
for the other casts too: near the end of cp_parser_functional_cast
instead of using the combined_loc only for the returned cp_expr we can
also pass it to build_functional_cast. Tested x86_64-linux.
Thanks, Paolo.
///
/gcc/cp
2019-12-04 Paolo Carlini
* typeck2.c (build_functional_cast): Add location_t parameter
and use it.
* cp-tree.h: Update declaration.
* parser.c (cp_parser_functional_cast): Adjust call.
* call.c (build_op_delete_call): Likewise.
(build_new_method_call_1): Likewise.
* decl.c (check_initializer): Likewise.
* pt.c (tsubst_copy_and_build): Likewise.
* semantics.c (finish_compound_literal): Likewise.
/libcc1
2019-12-04 Paolo Carlini
* libcp1plugin.cc (plugin_build_expression_list_expr): Adjust
build_functional_cast call.
/testsuite
2019-12-04 Paolo Carlini
* g++.dg/diagnostic/functional-cast-to-array-type-1.C: New.
* g++.dg/cpp0x/auto25.C: Check location(s) too.
* g++.dg/cpp0x/auto28.C: Likewise.
* g++.dg/init/reference2.C: Likewise.
* g++.dg/parse/template2.C: Likewise.
* g++.dg/template/error8.C: Likewise.
* g++.old-deja/g++.ns/crash3.C: Likewise.
* g++.old-deja/g++.ns/template7.C: Likewise.
* g++.old-deja/g++.pt/crash8.C: Likewise.
Index: gcc/cp/call.c
===
--- gcc/cp/call.c (revision 278959)
+++ gcc/cp/call.c (working copy)
@@ -6933,7 +6933,8 @@ build_op_delete_call (enum tree_code code, tree ad
rtype = cv_unqualified (rtype);
rtype = TYPE_POINTER_TO (rtype);
addr = cp_convert (rtype, oaddr, complain);
- destroying = build_functional_cast (destroying, NULL_TREE,
+ destroying = build_functional_cast (input_location,
+ destroying, NULL_TREE,
complain);
}
@@ -9997,7 +9998,8 @@ build_new_method_call_1 (tree instance, tree fns,
basetype, name))
inform (input_location, "for a function-style cast, remove the "
"redundant %<::%D%>", name);
- call = build_functional_cast (basetype, build_tree_list_vec (user_args),
+ call = build_functional_cast (input_location, basetype,
+ build_tree_list_vec (user_args),
complain);
return call;
}
Index: gcc/cp/cp-tree.h
===
--- gcc/cp/cp-tree.h(revision 278959)
+++ gcc/cp/cp-tree.h(working copy)
@@ -7681,7 +7681,8 @@ extern tree build_scoped_ref (tree,
tree, tree *
extern tree build_x_arrow (location_t, tree,
tsubst_flags_t);
extern tree build_m_component_ref (tree, tree, tsubst_flags_t);
-extern tree build_functional_cast (tree, tree, tsubst_flags_t);
+extern tree build_functional_cast (location_t, tree, tree,
+tsubst_flags_t);
extern tree add_exception_specifier(tree, tree, tsubst_flags_t);
extern tree merge_exception_specifiers (tree, tree);
Index: gcc/cp/decl.c
===
--- gcc/cp/decl.c (revision 278959)
+++ gcc/cp/decl.c (working copy)
@@ -6764,7 +6764,8 @@ check_initializer (tree decl, tree init, int flags
if (CLASS_TYPE_P (type)
&& (!init || TREE_CODE (init) == TREE_LIST))
{
- init = build_functional_cast (type, init, tf_none);
+ init = build_functional_cast (input_location, type,
+ init, tf_none);
if (TREE_CODE (init) == TARGET_EXPR)
TARGET_EXPR_DIRECT_INIT_P (init) = true;
}
Index: gcc/cp/parser.c
===
--- gcc/cp/parser.c (revision 278959)
+++ gcc/cp/parser.c (working copy)
@@ -29268,8 +29268,17 @@ cp_parser_functional_cast (cp_parser* parser, tree
release_tree_vector (vec);
}
- cast = build_functional_cast (type, expression_list,
+ /* Create a location of the form:
+ float(i)
+ ^~~~
+ with caret == start at the start of the type name,
+ finishing at the closing paren. */
+ location_t combined_loc = make_location (start_loc, start_loc,
+ parser->lexer);
+ cast = build_functional_cast (combined_loc, type, expression_list,
tf_warning_or_error
Re: [C++ Patch] A few more cp_expr_loc_or_input_loc and a diagnostic regression fix
Hi,
On 02/12/19 19:58, Jason Merrill wrote:
On 11/29/19 8:08 AM, Paolo Carlini wrote:
Hi,
a few more rather straightforward uses for cp_expr_loc_or_input_loc.
Additionally, while working on the latter, I noticed that, compared
to say, gcc-7, lately the code we have in cp_build_addr_expr_1 to
diagnose taking the address of 'main' often doesn't work anymore,
when the argument is wrapped in a location_wrapper. The below fixes
that by using tree_strip_any_location_wrapper in the DECL_MAIN_P
check, which works fine, but I can imagine various other ways to
solve the issue...
Maybe
location_t loc = cp_expr_loc_or_input_loc (arg);
STRIP_ANY_LOCATION_WRAPPER (arg);
at the top? In general I prefer the local variable to writing
cp_expr_loc_or_input_loc in lots of places, whether or not we then
strip wrappers.
Sure. In a few circumstances I hesitated because
cp_expr_loc_or_input_loc isn't really trivial, boils down to quite a few
conditionals, and adds a bit to the complexity of simple functions when
in fact no errors nor warnings are issued. But certainly calling it at
the top is often much cleaner. I changed both the functions.
However, using STRIP_ANY_LOCATION_WRAPPER at the beginning of
cp_build_addr_expr_1 doesn't seem straightforward, causes a few
regressions (wrong location for conversion/Wwrite-strings.C; even an ICE
for cpp1z/constexpr-lambda23.C; cpp1z/decomp48.C). The function doesn't
seem trivial from this point of view, there is already a localized
tree_strip_any_location_wrapper near the end, for
processing_template_decl. I would rather further investigate that kind
of simplification in a separate patch, beyond the regression fix.
(before I would like to improve the locations for all the various kinds
of cast, quite a few changes)
Thanks, Paolo.
///
Index: cp/typeck.c
===
--- cp/typeck.c (revision 278911)
+++ cp/typeck.c (working copy)
@@ -6061,6 +6061,7 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue
{
tree argtype;
tree val;
+ location_t loc;
if (!arg || error_operand_p (arg))
return error_mark_node;
@@ -6070,6 +6071,7 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue
return error_mark_node;
argtype = lvalue_type (arg);
+ loc = cp_expr_loc_or_input_loc (arg);
gcc_assert (!(identifier_p (arg) && IDENTIFIER_ANY_OP_P (arg)));
@@ -6103,12 +6105,14 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue
else if (current_class_type
&& TREE_OPERAND (arg, 0) == current_class_ref)
/* An expression like */
- permerror (input_location, "ISO C++ forbids taking the address of
an unqualified"
+ permerror (loc,
+ "ISO C++ forbids taking the address of an unqualified"
" or parenthesized non-static member function to form"
" a pointer to member function. Say %<&%T::%D%>",
base, name);
else
- permerror (input_location, "ISO C++ forbids taking the address of a
bound member"
+ permerror (loc,
+ "ISO C++ forbids taking the address of a bound member"
" function to form a pointer to member function."
" Say %<&%T::%D%>",
base, name);
@@ -6135,7 +6139,7 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue
if (kind == clk_none)
{
if (complain & tf_error)
- lvalue_error (cp_expr_loc_or_input_loc (arg), lv_addressof);
+ lvalue_error (loc, lv_addressof);
return error_mark_node;
}
if (strict_lvalue && (kind & (clk_rvalueref|clk_class)))
@@ -6143,8 +6147,7 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue
if (!(complain & tf_error))
return error_mark_node;
/* Make this a permerror because we used to accept it. */
- permerror (cp_expr_loc_or_input_loc (arg),
-"taking address of rvalue");
+ permerror (loc, "taking address of rvalue");
}
}
@@ -6154,13 +6157,13 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue
arg = build1 (CONVERT_EXPR, type, arg);
return arg;
}
- else if (pedantic && DECL_MAIN_P (arg))
+ else if (pedantic && DECL_MAIN_P (tree_strip_any_location_wrapper (arg)))
{
/* ARM $3.4 */
/* Apparently a lot of autoconf scripts for C++ packages do this,
so only complain if -Wpedantic. */
if (complain & (flag_pedantic_errors ? tf_error : tf_warning))
- pedwarn (input_location, OPT_Wpedantic,
+ pedwarn (loc, OPT_Wpedantic,
"ISO C++ forbids taking address of functio
[C++ Patch] A few more cp_expr_loc_or_input_loc and a diagnostic regression fix
Hi,
a few more rather straightforward uses for cp_expr_loc_or_input_loc.
Additionally, while working on the latter, I noticed that, compared to
say, gcc-7, lately the code we have in cp_build_addr_expr_1 to diagnose
taking the address of 'main' often doesn't work anymore, when the
argument is wrapped in a location_wrapper. The below fixes that by using
tree_strip_any_location_wrapper in the DECL_MAIN_P check, which works
fine, but I can imagine various other ways to solve the issue... Tested
x86_64-linux.
Thanks, Paolo.
//
/cp
2019-11-29 Paolo Carlini
* typeck.c (cp_build_addr_expr_1): Use cp_expr_loc_or_input_loc in a
few additional places.
(check_return_expr): Likewise.
* typeck.c (cp_build_addr_expr_1): Use tree_strip_any_location_wrapper
for address of main pedwarn.
/testsuite
2019-11-29 Paolo Carlini
* g++.dg/diagnostic/inconsistent-deduction-1.C: New.
* g++.dg/diagnostic/returning-a-value-1.C: Likewise.
* g++.dg/cpp0x/decltype3.C: Check location(s) too.
* g++.dg/cpp0x/decltype4.C: Likewise.
* g++.dg/cpp0x/lambda/lambda-deduce-ext-neg.C: Likewise.
* g++.dg/cpp2a/consteval13.C: Likewise.
* g++.dg/expr/pmf-1.C: Likewise.
* g++.dg/other/ptrmem2.C: Likewise.
* g++.dg/template/ptrmem17.C: Likewise.
* g++.old-deja/g++.bugs/900213_03.C: Likewise.
* g++.old-deja/g++.other/pmf7.C: Likewise.
* g++.old-deja/g++.other/ptrmem7.C: Likewise.
* g++.dg/diagnostic/main2.C: New.
Index: cp/typeck.c
===
--- cp/typeck.c (revision 278834)
+++ cp/typeck.c (working copy)
@@ -6103,12 +6103,14 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue
else if (current_class_type
&& TREE_OPERAND (arg, 0) == current_class_ref)
/* An expression like */
- permerror (input_location, "ISO C++ forbids taking the address of
an unqualified"
+ permerror (cp_expr_loc_or_input_loc (arg),
+ "ISO C++ forbids taking the address of an unqualified"
" or parenthesized non-static member function to form"
" a pointer to member function. Say %<&%T::%D%>",
base, name);
else
- permerror (input_location, "ISO C++ forbids taking the address of a
bound member"
+ permerror (cp_expr_loc_or_input_loc (arg),
+ "ISO C++ forbids taking the address of a bound member"
" function to form a pointer to member function."
" Say %<&%T::%D%>",
base, name);
@@ -6154,13 +6156,13 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue
arg = build1 (CONVERT_EXPR, type, arg);
return arg;
}
- else if (pedantic && DECL_MAIN_P (arg))
+ else if (pedantic && DECL_MAIN_P (tree_strip_any_location_wrapper (arg)))
{
/* ARM $3.4 */
/* Apparently a lot of autoconf scripts for C++ packages do this,
so only complain if -Wpedantic. */
if (complain & (flag_pedantic_errors ? tf_error : tf_warning))
- pedwarn (input_location, OPT_Wpedantic,
+ pedwarn (cp_expr_loc_or_input_loc (arg), OPT_Wpedantic,
"ISO C++ forbids taking address of function %<::main%>");
else if (flag_pedantic_errors)
return error_mark_node;
@@ -6218,7 +6220,8 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue
if (TYPE_REF_P (TREE_TYPE (t)))
{
if (complain & tf_error)
- error ("cannot create pointer to reference member %qD", t);
+ error_at (cp_expr_loc_or_input_loc (arg),
+ "cannot create pointer to reference member %qD", t);
return error_mark_node;
}
@@ -6254,8 +6257,9 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue
|| !DECL_IMMEDIATE_FUNCTION_P (current_function_decl)))
{
if (complain & tf_error)
- error ("taking address of an immediate function %qD",
- stripped_arg);
+ error_at (cp_expr_loc_or_input_loc (arg),
+ "taking address of an immediate function %qD",
+ stripped_arg);
return error_mark_node;
}
if (TREE_CODE (stripped_arg) == FUNCTION_DECL
@@ -9689,7 +9693,8 @@ check_return_expr (tree retval, bool *no_warning)
if (DECL_DESTRUCTOR_P (current_function_decl))
{
if (retval)
- error ("returning a value from a destructor");
+ error_at (cp_expr_loc_or_input_loc (retval),
+ "returning a value from a destructor");
[C++ Patch] More accurate locations in cp_build_unary_op and cp_build_compound_expr
Hi,
a bunch of straightforward tweaks: let's use the available location in
some additional places. Tested x86_64-linux, as usual.
Thanks, Paolo.
///
/cp
2019-11-15 Paolo Carlini
* typeck.c (cp_build_unary_op): Consistently use the accurate
location in seven additional diagnostic messages.
(cp_build_compound_expr): Use cp_expr_loc_or_input_loc in one place.
/testsuite
2019-11-15 Paolo Carlini
* g++.dg/cpp1z/bool-increment1.C: Test location(s) too.
* g++.dg/expr/bitfield3.C: Likewise.
* g++.dg/expr/bitfield4.C: Likewise.
* g++.dg/expr/bitfield5.C: Likewise.
* g++.dg/expr/bitfield6.C: Likewise.
* g++.dg/expr/bool1.C: Likewise.
* g++.dg/expr/bool2.C: Likewise.
* g++.dg/expr/bool3.C: Likewise.
* g++.dg/expr/bool4.C: Likewise.
* g++.dg/expr/lval3.C: Likewise.
* g++.dg/other/error18.C: Likewise.
* g++.dg/warn/Wpointer-arith-1.C: Likewise.
* g++.old-deja/g++.bugs/900212_01.C: Likewise.
* g++.old-deja/g++.bugs/900428_02.C: Likewise.
* g++.old-deja/g++.jason/rfg14.C: Likewise.
* g++.old-deja/g++.other/overload11.C: Likewise.
Index: cp/typeck.c
===
--- cp/typeck.c (revision 278692)
+++ cp/typeck.c (working copy)
@@ -6520,7 +6520,8 @@ cp_build_unary_op (enum tree_code code, tree xarg,
if (TREE_CODE (argtype) == ENUMERAL_TYPE)
{
if (complain & tf_error)
- permerror (input_location, (code == PREINCREMENT_EXPR || code ==
POSTINCREMENT_EXPR)
+ permerror (location, (code == PREINCREMENT_EXPR
+ || code == POSTINCREMENT_EXPR)
? G_("ISO C++ forbids incrementing an enum")
: G_("ISO C++ forbids decrementing an enum"));
else
@@ -6536,11 +6537,13 @@ cp_build_unary_op (enum tree_code code, tree xarg,
if (!COMPLETE_OR_VOID_TYPE_P (type))
{
if (complain & tf_error)
- error (((code == PREINCREMENT_EXPR
- || code == POSTINCREMENT_EXPR))
- ? G_("cannot increment a pointer to incomplete type
%qT")
- : G_("cannot decrement a pointer to incomplete type
%qT"),
- TREE_TYPE (argtype));
+ error_at (location, ((code == PREINCREMENT_EXPR
+ || code == POSTINCREMENT_EXPR))
+ ? G_("cannot increment a pointer to incomplete "
+"type %qT")
+ : G_("cannot decrement a pointer to incomplete "
+"type %qT"),
+ TREE_TYPE (argtype));
else
return error_mark_node;
}
@@ -6547,11 +6550,13 @@ cp_build_unary_op (enum tree_code code, tree xarg,
else if (!TYPE_PTROB_P (argtype))
{
if (complain & tf_error)
- pedwarn (input_location, OPT_Wpointer_arith,
+ pedwarn (location, OPT_Wpointer_arith,
(code == PREINCREMENT_EXPR
|| code == POSTINCREMENT_EXPR)
- ? G_("ISO C++ forbids incrementing a pointer of type
%qT")
- : G_("ISO C++ forbids decrementing a pointer of type
%qT"),
+ ? G_("ISO C++ forbids incrementing a pointer "
+ "of type %qT")
+ : G_("ISO C++ forbids decrementing a pointer "
+ "of type %qT"),
argtype);
else
return error_mark_node;
@@ -6597,8 +6602,9 @@ cp_build_unary_op (enum tree_code code, tree xarg,
if (code == POSTDECREMENT_EXPR || code == PREDECREMENT_EXPR)
{
if (complain & tf_error)
- error ("use of an operand of type %qT in % "
-"is forbidden", boolean_type_node);
+ error_at (location,
+ "use of an operand of type %qT in % "
+ "is forbidden", boolean_type_node);
return error_mark_node;
}
else
@@ -6606,16 +6612,18 @@ cp_build_unary_op (enum tree_code code, tree xarg,
if (cxx_dialect >= cxx17)
{
if (complain & tf_error)
- error ("use of an operand of type %qT in "
-"% is forbidden in C++17
[C++ Patch] Improve build_new_op_1, cp_build_indirect_ref_1, and cp_build_modify_expr locations
Hi,
I would say most of the changes are straightforward or mechanical.
Essentially, for build_new_op_1 and cp_build_modify_expr I'm simply
consistently using the available location argument; for
cp_build_indirect_ref_1 I'm adding the parameter but then using it in a
completely straightforward way. Minor nit: I wondered for a while if
cp_build_modify_expr should use cp_expr_loc_or_loc more - normally the
passed loc points to the '=' - but eventually, given the actual texts of
the messages, I used it only in one place, for "void value not ignored
as it ought to be" which is mostly about the type of 'rhs'. All the
other messages in one way or the other talk about both sides (the
primary clang caret appears to agree).
Tested x86_64-linux.
Thanks, Paolo.
//
/gcc
2019-11-20 Paolo Carlini
* typeck.c (cp_build_indirect_ref_1): Add location_t parameter
and use it in error messages.
(build_x_indirect_ref): Adjust call.
(build_indirect_ref): Likewise.
(cp_build_fold_indirect_ref): Likewise.
(cp_build_array_ref): Likewise.
* call.c (build_new_op_1): Likewise.
* semantics.c (finish_omp_clauses): Likewise.
(finish_omp_depobj): Likewise.
* typeck2.c (build_x_arrow): Likewise.
* cp-tree.h (cp_build_indirect_ref): Update declaration.
* call.c (build_new_op_1): Use location argument in warning_at.
* typeck.c (cp_build_modify_expr): Consistently use the
location_t argument.
/libcc1
2019-11-20 Paolo Carlini
* libcp1plugin.cc (plugin_pragma_push_user_expression): Update
cp_build_indirect_ref call.
/testsuite
2019-11-20 Paolo Carlini
* g++.dg/diagnostic/base-operand-non-pointer-1.C: New.
* g++.dg/pr53055.C: Check location too.
* g++.old-deja/g++.bugs/900213_02.C: Likewise.
* g++.old-deja/g++.bugs/900215_02.C: Likewise.
* g++.old-deja/g++.other/badarrow.C: Likewise.
* g++.old-deja/g++.other/deref1.C: Likewise.
* g++.dg/warn/Wenum-compare.C: Check location too.
* g++.dg/cpp0x/initlist26.C: Check location too.
* g++.dg/cpp0x/initlist28.C: Likewise.
* g++.dg/cpp0x/initlist29.C: Likewise.
* g++.dg/cpp0x/initlist33.C: Likewise.
* g++.dg/expr/string-2.C: Likewise.
* g++.dg/other/ptrmem5.C: Likewise.
* g++.old-deja/g++.benjamin/14664-1.C: Likewise.
* g++.old-deja/g++.benjamin/14664-2.C: Likewise.
* g++.old-deja/g++.brendan/init12.C: Likewise.
* g++.old-deja/g++.bugs/900324_04.C: Likewise.
* g++.old-deja/g++.ext/array1.C: Likewise.
* g++.old-deja/g++.jason/rfg17.C: Likewise.
Index: cp/call.c
===
--- cp/call.c (revision 278549)
+++ cp/call.c (working copy)
@@ -6354,11 +6354,9 @@ build_new_op_1 (const op_location_t , enum tre
&& (TYPE_MAIN_VARIANT (arg1_type)
!= TYPE_MAIN_VARIANT (arg2_type))
&& (complain & tf_warning))
- {
- warning (OPT_Wenum_compare,
- "comparison between %q#T and %q#T",
- arg1_type, arg2_type);
- }
+ warning_at (loc, OPT_Wenum_compare,
+ "comparison between %q#T and %q#T",
+ arg1_type, arg2_type);
break;
default:
break;
@@ -6416,7 +6414,7 @@ build_new_op_1 (const op_location_t , enum tre
return cp_build_modify_expr (loc, arg1, code2, arg2, complain);
case INDIRECT_REF:
- return cp_build_indirect_ref (arg1, RO_UNARY_STAR, complain);
+ return cp_build_indirect_ref (loc, arg1, RO_UNARY_STAR, complain);
case TRUTH_ANDIF_EXPR:
case TRUTH_ORIF_EXPR:
@@ -6472,8 +6470,9 @@ build_new_op_1 (const op_location_t , enum tre
return cp_build_array_ref (input_location, arg1, arg2, complain);
case MEMBER_REF:
- return build_m_component_ref (cp_build_indirect_ref (arg1,
RO_ARROW_STAR,
- complain),
+ return build_m_component_ref (cp_build_indirect_ref (loc, arg1,
+ RO_ARROW_STAR,
+ complain),
arg2, complain);
/* The caller will deal with these. */
Index: cp/cp-tree.h
===
--- cp/cp-tree.h(revision 278549)
+++ cp/cp-tree.h(working copy)
@@ -7482,9 +7482,11 @@ extern tree build_class_member_access_expr (c
extern tree finish_class_member_access_expr (cp_expr, tree, bool,
tsubst_flags_t);
extern tree bu
[C++ Patch] Avoid redundant error messages from build_x_arrow
Hi,
while working on improving the locations of cp_build_indirect_ref_1 &
co, I noticed this nit which seems a separate issue.
In a nutshell, at variance with many other cases, in build_x_arrow we
don't immediately check for error_mark_node the return value of
decay_conversion. Then, for the testcase, after a sensible:
error: invalid use of member function ‘C* C::f()’ (did you forget the
‘()’ ?)
we also issue:
error: base operand of ‘->’ is not a pointer
which is certainly redundant and a bit misleading, is talking about the
'f' mentioned in the first message. The amended behavior is also
consistent with EDG and CLANG.
Tested x86_64-linux, as usual.
Thanks, Paolo.
//
/gcc
2019-11-20 Paolo Carlini
* typeck2.c (build_x_arrow): Early return if decay_conversion
returns error_mark_node.
/testsuite
2019-11-20 Paolo Carlini
* g++.dg/parse/error43.C: Adjust expected error.
Index: cp/typeck2.c
===
--- cp/typeck2.c(revision 278499)
+++ cp/typeck2.c(working copy)
@@ -2044,7 +2044,11 @@ build_x_arrow (location_t loc, tree expr, tsubst_f
last_rval = convert_from_reference (last_rval);
}
else
-last_rval = decay_conversion (expr, complain);
+{
+ last_rval = decay_conversion (expr, complain);
+ if (last_rval == error_mark_node)
+ return error_mark_node;
+}
if (TYPE_PTR_P (TREE_TYPE (last_rval)))
{
Index: testsuite/g++.dg/parse/error43.C
===
--- testsuite/g++.dg/parse/error43.C(revision 278499)
+++ testsuite/g++.dg/parse/error43.C(working copy)
@@ -2,4 +2,4 @@
// { dg-options "" }
class C { public: C* f(); int get(); };
-int f(C* p) { return p->f->get(); } // { dg-error "forget the '\\(\\)'|base
operand" }
+int f(C* p) { return p->f->get(); } // { dg-error "25:invalid use of member
function" }
[C++ Patch] Avoid duplicate warning
Hi,
functions like c_common_truthvalue_conversion are shared with the C
front-end thus don't get a tsubst_flags_t argument. It seems clear that
long term we have to do something about those but in the meanwhile we
have been using warning sentinels which, along the paths which certainly
must have all the warnings suppressed, work well for now and cannot
cause regressions. Here I propose to add (at least) one more to
ocp_convert since I have a straightforward testcase. Note sue if we want
to proactively add, say, one for warn_parentheses too.
Tested x86_64-linux.
Thanks, Paolo.
///
/cp
2019-11-18 Paolo Carlini
* cvt.c (ocp_convert): Use additional warning sentinel.
/testsuite
2019-11-18 Paolo Carlini
* g++.dg/warn/multiple-sign-compare-warn-1.C: New.
Index: cp/cvt.c
===
--- cp/cvt.c(revision 278407)
+++ cp/cvt.c(working copy)
@@ -847,6 +847,7 @@ ocp_convert (tree type, tree expr, int convtype, i
/* Prevent bogus -Wint-in-bool-context warnings coming
from c_common_truthvalue_conversion down the line. */
warning_sentinel w (warn_int_in_bool_context);
+ warning_sentinel c (warn_sign_compare);
return cp_truthvalue_conversion (e, complain);
}
}
Index: testsuite/g++.dg/warn/multiple-sign-compare-warn-1.C
===
--- testsuite/g++.dg/warn/multiple-sign-compare-warn-1.C(nonexistent)
+++ testsuite/g++.dg/warn/multiple-sign-compare-warn-1.C(working copy)
@@ -0,0 +1,11 @@
+// { dg-options "-Wsign-compare" }
+
+int foo()
+{
+ unsigned char b = '1';
+
+ bool x = ~b; // { dg-bogus "promoted bitwise complement of an unsigned value
is always nonzero.*promoted bitwise complement of an unsigned value is always
nonzero" }
+ // { dg-warning "promoted bitwise complement of an unsigned value is always
nonzero" "" { target *-*-* } .-1 }
+
+ return 0;
+}
Re: [C++ Patch] Use cp_expr_loc_or_input_loc in a few additional typeck.c places
Hi again,
On 14/11/19 12:09, Paolo Carlini wrote:
Hi,
tested x86_64-linux.
Instead of sending a separate patch, the below has two additional uses.
Tested as usual.
Thanks, Paolo.
///
/cp
2019-11-14 Paolo Carlini
* typeck.c (cp_build_addr_expr_1): Use cp_expr_loc_or_input_loc
in three places.
(cxx_sizeof_expr): Use it in one additional place.
(cxx_alignof_expr): Likewise.
(lvalue_or_else): Likewise.
/testsuite
2019-11-14 Paolo Carlini
* g++.dg/cpp0x/addressof2.C: Test locations too.
* g++.dg/cpp0x/rv-lvalue-req.C: Likewise.
* g++.dg/expr/crash2.C: Likewise.
* g++.dg/expr/lval1.C: Likewise.
* g++.dg/expr/unary2.C: Likewise.
* g++.dg/ext/lvaddr.C: Likewise.
* g++.dg/ext/lvalue1.C: Likewise.
* g++.dg/tree-ssa/pr20280.C: Likewise.
* g++.dg/warn/Wplacement-new-size.C: Likewise.
* g++.old-deja/g++.brendan/alignof.C: Likewise.
* g++.old-deja/g++.brendan/sizeof2.C: Likewise.
* g++.old-deja/g++.law/temps1.C: Likewise.
Index: cp/typeck.c
===
--- cp/typeck.c (revision 278253)
+++ cp/typeck.c (working copy)
@@ -1765,7 +1765,8 @@ cxx_sizeof_expr (tree e, tsubst_flags_t complain)
if (bitfield_p (e))
{
if (complain & tf_error)
-error ("invalid application of % to a bit-field");
+ error_at (cp_expr_loc_or_input_loc (e),
+ "invalid application of % to a bit-field");
else
return error_mark_node;
e = char_type_node;
@@ -1825,7 +1826,8 @@ cxx_alignof_expr (tree e, tsubst_flags_t complain)
else if (bitfield_p (e))
{
if (complain & tf_error)
-error ("invalid application of %<__alignof%> to a bit-field");
+ error_at (cp_expr_loc_or_input_loc (e),
+ "invalid application of %<__alignof%> to a bit-field");
else
return error_mark_node;
t = size_one_node;
@@ -6126,7 +6128,7 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue
if (kind == clk_none)
{
if (complain & tf_error)
- lvalue_error (input_location, lv_addressof);
+ lvalue_error (cp_expr_loc_or_input_loc (arg), lv_addressof);
return error_mark_node;
}
if (strict_lvalue && (kind & (clk_rvalueref|clk_class)))
@@ -6134,7 +6136,8 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue
if (!(complain & tf_error))
return error_mark_node;
/* Make this a permerror because we used to accept it. */
- permerror (input_location, "taking address of rvalue");
+ permerror (cp_expr_loc_or_input_loc (arg),
+"taking address of rvalue");
}
}
@@ -6228,7 +6231,8 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue
if (bitfield_p (arg))
{
if (complain & tf_error)
- error ("attempt to take address of bit-field");
+ error_at (cp_expr_loc_or_input_loc (arg),
+ "attempt to take address of bit-field");
return error_mark_node;
}
@@ -10431,7 +10435,7 @@ lvalue_or_else (tree ref, enum lvalue_use use, tsu
if (kind == clk_none)
{
if (complain & tf_error)
- lvalue_error (input_location, use);
+ lvalue_error (cp_expr_loc_or_input_loc (ref), use);
return 0;
}
else if (kind & (clk_rvalueref|clk_class))
Index: testsuite/g++.dg/cpp0x/addressof2.C
===
--- testsuite/g++.dg/cpp0x/addressof2.C (revision 278253)
+++ testsuite/g++.dg/cpp0x/addressof2.C (working copy)
@@ -8,19 +8,19 @@ addressof (T ) noexcept
return __builtin_addressof (x);
}
-auto a = __builtin_addressof (1); // { dg-error "lvalue required
as unary" }
-auto b = addressof (1);// { dg-error "cannot
bind non-const lvalue reference of type" }
+auto a = __builtin_addressof (1); // { dg-error "31:lvalue
required as unary" }
+auto b = addressof (1);// { dg-error
"21:cannot bind non-const lvalue reference of type" }
struct S { int s : 5; int t; void foo (); } s;
auto c = __builtin_addressof (s);
auto d = addressof (s);
-auto e = __builtin_addressof (s.s);// { dg-error "attempt to take
address of bit-field" }
-auto f = addressof (s.s); // { dg-error "cannot bind
bit-field" }
-auto g = __builtin_addressof (S{});// { dg-error "taking address
of rvalue" }
-auto h = addressof (S{}); // { dg-error "cannot bind
non-const lvalue reference of type" }
-auto i = __builtin_addressof (S::t); // { dg-error &
[C++ Patch] Use cp_expr_loc_or_input_loc in a few additional typeck.c places
Hi,
tested x86_64-linux.
Thanks, Paolo.
///
/cp
2019-11-14 Paolo Carlini
* typeck.c (cp_build_addr_expr_1): Use cp_expr_loc_or_input_loc
in three places.
(lvalue_or_else): Use it in one place.
/testsuite
2019-11-14 Paolo Carlini
* g++.dg/cpp0x/addressof2.C: Test locations too.
* g++.dg/cpp0x/rv-lvalue-req.C: Likewise.
* g++.dg/expr/crash2.C: Likewise.
* g++.dg/expr/lval1.C: Likewise.
* g++.dg/expr/unary2.C: Likewise.
* g++.dg/ext/lvaddr.C: Likewise.
* g++.dg/ext/lvalue1.C: Likewise.
* g++.dg/tree-ssa/pr20280.C: Likewise.
* g++.dg/warn/Wplacement-new-size.C: Likewise.
* g++.old-deja/g++.law/temps1.C: Likewise.
Index: cp/typeck.c
===
--- cp/typeck.c (revision 278216)
+++ cp/typeck.c (working copy)
@@ -6126,7 +6126,7 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue
if (kind == clk_none)
{
if (complain & tf_error)
- lvalue_error (input_location, lv_addressof);
+ lvalue_error (cp_expr_loc_or_input_loc (arg), lv_addressof);
return error_mark_node;
}
if (strict_lvalue && (kind & (clk_rvalueref|clk_class)))
@@ -6134,7 +6134,8 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue
if (!(complain & tf_error))
return error_mark_node;
/* Make this a permerror because we used to accept it. */
- permerror (input_location, "taking address of rvalue");
+ permerror (cp_expr_loc_or_input_loc (arg),
+"taking address of rvalue");
}
}
@@ -6228,7 +6229,8 @@ cp_build_addr_expr_1 (tree arg, bool strict_lvalue
if (bitfield_p (arg))
{
if (complain & tf_error)
- error ("attempt to take address of bit-field");
+ error_at (cp_expr_loc_or_input_loc (arg),
+ "attempt to take address of bit-field");
return error_mark_node;
}
@@ -10431,7 +10433,7 @@ lvalue_or_else (tree ref, enum lvalue_use use, tsu
if (kind == clk_none)
{
if (complain & tf_error)
- lvalue_error (input_location, use);
+ lvalue_error (cp_expr_loc_or_input_loc (ref), use);
return 0;
}
else if (kind & (clk_rvalueref|clk_class))
Index: testsuite/g++.dg/cpp0x/addressof2.C
===
--- testsuite/g++.dg/cpp0x/addressof2.C (revision 278216)
+++ testsuite/g++.dg/cpp0x/addressof2.C (working copy)
@@ -8,19 +8,19 @@ addressof (T ) noexcept
return __builtin_addressof (x);
}
-auto a = __builtin_addressof (1); // { dg-error "lvalue required
as unary" }
-auto b = addressof (1);// { dg-error "cannot
bind non-const lvalue reference of type" }
+auto a = __builtin_addressof (1); // { dg-error "31:lvalue
required as unary" }
+auto b = addressof (1);// { dg-error
"21:cannot bind non-const lvalue reference of type" }
struct S { int s : 5; int t; void foo (); } s;
auto c = __builtin_addressof (s);
auto d = addressof (s);
-auto e = __builtin_addressof (s.s);// { dg-error "attempt to take
address of bit-field" }
-auto f = addressof (s.s); // { dg-error "cannot bind
bit-field" }
-auto g = __builtin_addressof (S{});// { dg-error "taking address
of rvalue" }
-auto h = addressof (S{}); // { dg-error "cannot bind
non-const lvalue reference of type" }
-auto i = __builtin_addressof (S::t); // { dg-error "invalid use of
non-static data member" }
-auto j = __builtin_addressof (S::foo); // { dg-error "invalid use of
non-static member function" }
+auto e = __builtin_addressof (s.s);// { dg-error "33:attempt to
take address of bit-field" }
+auto f = addressof (s.s); // { dg-error "23:cannot bind
bit-field" }
+auto g = __builtin_addressof (S{});// { dg-error "31:taking
address of rvalue" }
+auto h = addressof (S{}); // { dg-error "21:cannot bind
non-const lvalue reference of type" }
+auto i = __builtin_addressof (S::t); // { dg-error "34:invalid use
of non-static data member" }
+auto j = __builtin_addressof (S::foo); // { dg-error "34:invalid use
of non-static member function" }
void
foo (bool b)
@@ -28,6 +28,6 @@ foo (bool b)
lab:;
char c;
long long int d;
- auto k = __builtin_addressof (lab); // { dg-error "was not declared
in this scope" }
- auto l = __builtin_addressof (b ? c : d);// { dg-error "lvalue required
as unary"
[C++ Patch] Improve cp_truthvalue_conversion locations and more
Hi,
a few days ago I noticed that we weren't doing the right thing
location-wise for the first test of g++.dg/warn/Waddress-1.C: it seems
clear that the ultimate reason is that we didn't pass an accurate
location to build_binary_op called from cp_truthvalue_conversion. When I
fixed that, I noticed that for testcases like warn/Walways-true-3.C we
were issuing many duplicate warnings and I traced that to ocp_convert
not using its tsubst_flags_t argument - which can come from
cp_convert_and_check as tf_none - when calling cp_truthvalue_conversion.
Thus I also added a tusbst_flags_t parameter to the latter and
everything looks finally good for these testcases.
Tested x86_64-linux.
Thanks, Paolo.
/cp
2019-11-12 Paolo Carlini
* typeck.c (cp_truthvalue_conversion): Add tsubst_flags_t parameter
and use it in calls; also pass the location_t of the expression to
cp_build_binary_op and c_common_truthvalue_conversion.
* rtti.c (build_dynamic_cast_1): Adjust call.
* cvt.c (ocp_convert): Likewise.
* cp-gimplify.c (cp_fold): Likewise.
* cp-tree.h (cp_truthvalue_conversion): Update declaration.
/testsuite
2019-11-12 Paolo Carlini
* g++.dg/warn/Walways-true-1.C: Check locations too.
* g++.dg/warn/Walways-true-2.C: Likewise.
* g++.dg/warn/Walways-true-3.C: Likewise.
* g++.dg/warn/Waddress-1.C: Check additional location.
Index: cp/cp-gimplify.c
===
--- cp/cp-gimplify.c(revision 278059)
+++ cp/cp-gimplify.c(working copy)
@@ -2573,9 +2573,9 @@ cp_fold (tree x)
{
warning_sentinel s (warn_int_in_bool_context);
if (!VOID_TYPE_P (TREE_TYPE (op1)))
- op1 = cp_truthvalue_conversion (op1);
+ op1 = cp_truthvalue_conversion (op1, tf_warning_or_error);
if (!VOID_TYPE_P (TREE_TYPE (op2)))
- op2 = cp_truthvalue_conversion (op2);
+ op2 = cp_truthvalue_conversion (op2, tf_warning_or_error);
}
else if (VOID_TYPE_P (TREE_TYPE (x)))
{
Index: cp/cp-tree.h
===
--- cp/cp-tree.h(revision 278059)
+++ cp/cp-tree.h(working copy)
@@ -7445,7 +7445,7 @@ enum compare_bounds_t { bounds_none, bounds_either
extern bool cxx_mark_addressable (tree, bool = false);
extern int string_conv_p (const_tree, const_tree, int);
-extern tree cp_truthvalue_conversion (tree);
+extern tree cp_truthvalue_conversion (tree, tsubst_flags_t);
extern tree contextual_conv_bool (tree, tsubst_flags_t);
extern tree condition_conversion (tree);
extern tree require_complete_type (tree);
Index: cp/cvt.c
===
--- cp/cvt.c(revision 278059)
+++ cp/cvt.c(working copy)
@@ -841,13 +841,13 @@ ocp_convert (tree type, tree expr, int convtype, i
if (SCOPED_ENUM_P (intype) && (convtype & CONV_STATIC))
e = build_nop (ENUM_UNDERLYING_TYPE (intype), e);
if (complain & tf_warning)
- return cp_truthvalue_conversion (e);
+ return cp_truthvalue_conversion (e, complain);
else
{
/* Prevent bogus -Wint-in-bool-context warnings coming
from c_common_truthvalue_conversion down the line. */
warning_sentinel w (warn_int_in_bool_context);
- return cp_truthvalue_conversion (e);
+ return cp_truthvalue_conversion (e, complain);
}
}
Index: cp/rtti.c
===
--- cp/rtti.c (revision 278059)
+++ cp/rtti.c (working copy)
@@ -782,7 +782,7 @@ build_dynamic_cast_1 (tree type, tree expr, tsubst
tree neq;
result = save_expr (result);
- neq = cp_truthvalue_conversion (result);
+ neq = cp_truthvalue_conversion (result, complain);
return cp_convert (type,
build3 (COND_EXPR, TREE_TYPE (result),
neq, result, bad), complain);
Index: cp/typeck.c
===
--- cp/typeck.c (revision 278059)
+++ cp/typeck.c (working copy)
@@ -5975,15 +5975,16 @@ cp_build_addressof (location_t loc, tree arg, tsub
-1. */
tree
-cp_truthvalue_conversion (tree expr)
+cp_truthvalue_conversion (tree expr, tsubst_flags_t complain)
{
tree type = TREE_TYPE (expr);
+ location_t loc = cp_expr_loc_or_input_loc (expr);
if (TYPE_PTR_OR_PTRMEM_P (type)
/* Avoid ICE on invalid use of non-static member function. */
|| TREE_CODE (expr) == FUNCTION_DECL)
-return build_binary_op (input_location, NE_EXPR, expr, nullptr_node, tr
Re: [C++ Patch] Add location parameter to composite_pointer_type and use it
Hi again,
On 31/10/19 16:56, Paolo Carlini wrote:
Hi,
yesterday I noticed that we want to forward the location parameter of
cp_build_binary_op to composite_pointer_type and then
composite_pointer_error. Note, for the time being at least, this is
always for CPO_COMPARISON, the other two composite_pointer_operation
cases aren't involved - and the various functions (eg,
common_pointer_type) forward input_location - but I'm consistently
changing all the pedwarn, emit_diagnostic, and error_at anyway to use
the location.
In fact, build_conditional_expr_1, which passes CPO_CONDITIONAL_EXPR,
has available a suitable location, thus we can extend my previous patch
and forward that too instead of input_location. Tested as usual
x86_64-linux.
Thanks, Paolo.
/
/cp
2019-11-01 Paolo Carlini
* typeck.c (composite_pointer_type): Add a const op_location_t&
parameter and use it in diagnostics.
(composite_pointer_error): Likewise.
(composite_pointer_type_r): Add a const op_location_t&
parameter and forward it.
(cp_build_binary_op): Adjust calls.
(common_pointer_type): Likewise.
* call.c (add_builtin_candidate): Likewise.
(build_conditional_expr_1): Likewise.
* cp-tree.h (composite_pointer_type): Update declaration.
* typeck.c (cxx_sizeof_expr): Use cp_expr_loc_or_input_loc
in permerror.
(cxx_alignof_expr): Likewise.
(lvalue_or_else): Likewise.
/testsuite
2019-11-01 Paolo Carlini
* g++.dg/conversion/ptrmem9.C: Check location.
* g++.dg/expr/cond2.C: Likewise.
* g++.dg/warn/Waddress-1.C: Check locations.
* g++.old-deja/g++.bugs/900324_02.C: Check location.
* g++.old-deja/g++.jason/rfg20.C: Likewise.
* g++.old-deja/g++.law/typeck1.C: Likewise.
* g++.old-deja/g++.rfg/00321_01-.C: Likewise.
* g++.old-deja/g++.rfg/00324_02-.C: Likewise.
* g++.dg/diagnostic/alignof1.C: New.
* g++.dg/expr/sizeof1.C: Check location.
* g++.dg/cpp0x/rv-lvalue-req.C: Check locations.
Index: cp/call.c
===
--- cp/call.c (revision 277705)
+++ cp/call.c (working copy)
@@ -3029,7 +3029,8 @@ add_builtin_candidate (struct z_candidate **candid
{
if (TYPE_PTR_OR_PTRMEM_P (type1))
{
- tree cptype = composite_pointer_type (type1, type2,
+ tree cptype = composite_pointer_type (input_location,
+ type1, type2,
error_mark_node,
error_mark_node,
CPO_CONVERSION,
@@ -5553,7 +5554,8 @@ build_conditional_expr_1 (const op_location_t
|| (TYPE_PTRDATAMEM_P (arg2_type) && TYPE_PTRDATAMEM_P (arg3_type))
|| (TYPE_PTRMEMFUNC_P (arg2_type) && TYPE_PTRMEMFUNC_P (arg3_type)))
{
- result_type = composite_pointer_type (arg2_type, arg3_type, arg2,
+ result_type = composite_pointer_type (loc,
+ arg2_type, arg3_type, arg2,
arg3, CPO_CONDITIONAL_EXPR,
complain);
if (result_type == error_mark_node)
Index: cp/cp-tree.h
===
--- cp/cp-tree.h(revision 277705)
+++ cp/cp-tree.h(working copy)
@@ -7509,7 +7509,8 @@ extern tree build_ptrmemfunc1 (tree,
tree, tree)
extern void expand_ptrmemfunc_cst (tree, tree *, tree *);
extern tree type_after_usual_arithmetic_conversions (tree, tree);
extern tree common_pointer_type (tree, tree);
-extern tree composite_pointer_type (tree, tree, tree, tree,
+extern tree composite_pointer_type (const op_location_t &,
+tree, tree, tree, tree,
composite_pointer_operation,
tsubst_flags_t);
extern tree merge_types(tree, tree);
Index: cp/typeck.c
===
--- cp/typeck.c (revision 277705)
+++ cp/typeck.c (working copy)
@@ -450,25 +450,26 @@ type_after_usual_arithmetic_conversions (tree t1,
}
static void
-composite_pointer_error (diagnostic_t kind, tree t1, tree t2,
+composite_pointer_error (const op_location_t ,
+diagnostic_t kind, tree t1, tree t2,
composite_pointer_operation operation)
{
switch (operation)
{
case CPO_COMPARISON:
- emit_diagnostic (kind, input_location, 0,
+ emit_diagnostic (kind, location, 0,
"comparison between "
[C++ Patch] Add location parameter to composite_pointer_type and use it
Hi,
yesterday I noticed that we want to forward the location parameter of
cp_build_binary_op to composite_pointer_type and then
composite_pointer_error. Note, for the time being at least, this is
always for CPO_COMPARISON, the other two composite_pointer_operation
cases aren't involved - and the various functions (eg,
common_pointer_type) forward input_location - but I'm consistently
changing all the pedwarn, emit_diagnostic, and error_at anyway to use
the location. Additionally, I had in my tree some additional uses for
cp_expr_loc_or_input_loc.
Tested x86_64-linux.
Thanks, Paolo.
/cp
2019-10-31 Paolo Carlini
* typeck.c (composite_pointer_type): Add a const op_location_t&
parameter and use it in diagnostics.
(composite_pointer_error): Likewise.
(composite_pointer_type_r): Add a const op_location_t&
parameter and forward it.
(cp_build_binary_op): Adjust calls.
(common_pointer_type): Likewise.
* call.c (add_builtin_candidate): Likewise.
(build_conditional_expr_1): Likewise.
* cp-tree.h (composite_pointer_type): Update declaration.
* typeck.c (cxx_sizeof_expr): Use cp_expr_loc_or_input_loc
in permerror.
(cxx_alignof_expr): Likewise.
(lvalue_or_else): Likewise.
/testsuite
2019-10-31 Paolo Carlini
* g++.dg/conversion/ptrmem9.C: Check location.
* g++.dg/warn/Waddress-1.C: Check locations.
* g++.old-deja/g++.jason/rfg20.C: Likewise.
* g++.old-deja/g++.law/typeck1.C: Likewise.
* g++.old-deja/g++.rfg/00321_01-.C: Likewise.
* g++.dg/diagnostic/alignof1.C: New.
* g++.dg/expr/sizeof1.C: Check location.
* g++.dg/cpp0x/rv-lvalue-req.C: Check locations.
Index: cp/call.c
===
--- cp/call.c (revision 277657)
+++ cp/call.c (working copy)
@@ -3029,7 +3029,8 @@ add_builtin_candidate (struct z_candidate **candid
{
if (TYPE_PTR_OR_PTRMEM_P (type1))
{
- tree cptype = composite_pointer_type (type1, type2,
+ tree cptype = composite_pointer_type (input_location,
+ type1, type2,
error_mark_node,
error_mark_node,
CPO_CONVERSION,
@@ -5553,7 +5554,8 @@ build_conditional_expr_1 (const op_location_t
|| (TYPE_PTRDATAMEM_P (arg2_type) && TYPE_PTRDATAMEM_P (arg3_type))
|| (TYPE_PTRMEMFUNC_P (arg2_type) && TYPE_PTRMEMFUNC_P (arg3_type)))
{
- result_type = composite_pointer_type (arg2_type, arg3_type, arg2,
+ result_type = composite_pointer_type (input_location,
+ arg2_type, arg3_type, arg2,
arg3, CPO_CONDITIONAL_EXPR,
complain);
if (result_type == error_mark_node)
Index: cp/cp-tree.h
===
--- cp/cp-tree.h(revision 277657)
+++ cp/cp-tree.h(working copy)
@@ -7509,7 +7509,8 @@ extern tree build_ptrmemfunc1 (tree,
tree, tree)
extern void expand_ptrmemfunc_cst (tree, tree *, tree *);
extern tree type_after_usual_arithmetic_conversions (tree, tree);
extern tree common_pointer_type (tree, tree);
-extern tree composite_pointer_type (tree, tree, tree, tree,
+extern tree composite_pointer_type (const op_location_t &,
+tree, tree, tree, tree,
composite_pointer_operation,
tsubst_flags_t);
extern tree merge_types(tree, tree);
Index: cp/typeck.c
===
--- cp/typeck.c (revision 277657)
+++ cp/typeck.c (working copy)
@@ -450,25 +450,26 @@ type_after_usual_arithmetic_conversions (tree t1,
}
static void
-composite_pointer_error (diagnostic_t kind, tree t1, tree t2,
+composite_pointer_error (const op_location_t ,
+diagnostic_t kind, tree t1, tree t2,
composite_pointer_operation operation)
{
switch (operation)
{
case CPO_COMPARISON:
- emit_diagnostic (kind, input_location, 0,
+ emit_diagnostic (kind, location, 0,
"comparison between "
"distinct pointer types %qT and %qT lacks a cast",
t1, t2);
break;
case CPO_CONVERSION:
- emit_diagnostic (kind, input_location, 0,
+ emit_diagnostic (kind, location, 0,
"conversion between "
[C++ Patch] Prefer error + inform in four typeck.c places
Hi,
some additional straightforward bits in typeck.c, which I noticed when I
went through the cp_build_binary_op callers. Tested x86_64-linux.
Thanks, Paolo.
//
/cp
2019-10-24 Paolo Carlini
* typeck.c (cp_build_modify_expr): Prefer error + inform to
error + error in one place.
(get_delta_difference_1): Likewise.
(get_delta_difference): Likewise, in two places.
/testsuite
2019-10-24 Paolo Carlini
* g++.dg/conversion/ptrmem2.C: Adjust for error + inform.
* g++.dg/gomp/tpl-atomic-2.C: Likewise.
Index: cp/typeck.c
===
--- cp/typeck.c (revision 277366)
+++ cp/typeck.c (working copy)
@@ -8358,8 +8358,8 @@ cp_build_modify_expr (location_t loc, tree lhs, en
if (newrhs == error_mark_node)
{
if (complain & tf_error)
- error (" in evaluation of %<%Q(%#T, %#T)%>", modifycode,
- TREE_TYPE (lhs), TREE_TYPE (rhs));
+ inform (loc, " in evaluation of %<%Q(%#T, %#T)%>",
+ modifycode, TREE_TYPE (lhs), TREE_TYPE (rhs));
return error_mark_node;
}
@@ -8594,7 +8594,7 @@ get_delta_difference_1 (tree from, tree to, bool c
if (!(complain & tf_error))
return error_mark_node;
- error (" in pointer to member function conversion");
+ inform (input_location, " in pointer to member function conversion");
return size_zero_node;
}
else if (binfo)
@@ -8655,7 +8655,7 @@ get_delta_difference (tree from, tree to,
return error_mark_node;
error_not_base_type (from, to);
- error (" in pointer to member conversion");
+ inform (input_location, " in pointer to member conversion");
result = size_zero_node;
}
else
@@ -8674,7 +8674,7 @@ get_delta_difference (tree from, tree to,
return error_mark_node;
error_not_base_type (from, to);
- error (" in pointer to member conversion");
+ inform (input_location, " in pointer to member conversion");
result = size_zero_node;
}
}
Index: testsuite/g++.dg/conversion/ptrmem2.C
===
--- testsuite/g++.dg/conversion/ptrmem2.C (revision 277366)
+++ testsuite/g++.dg/conversion/ptrmem2.C (working copy)
@@ -15,16 +15,20 @@ int B::*p1 = static_cast(::x);
int D::*p2 = static_cast(::x);
// Virtual base class.
-int V::*p3 = static_cast(::x); // { dg-error "" }
-int D::*p4 = static_cast(::x); // { dg-error "" }
+int V::*p3 = static_cast(::x); // { dg-error "virtual base" }
+int D::*p4 = static_cast(::x); // { dg-error "virtual base" }
// Inaccessible base class.
-int P::*p5 = static_cast(::x); // { dg-error "" }
-int D::*p6 = static_cast(::x); // { dg-error "" }
+int P::*p5 = static_cast(::x); // { dg-error "inaccessible base" }
+// { dg-message "pointer to member function" "" { target *-*-* } .-1 }
+int D::*p6 = static_cast(::x); // { dg-error "inaccessible base" }
+// { dg-message "pointer to member function" "" { target *-*-* } .-1 }
// Ambiguous base class.
-int A::*p7 = static_cast(::x); // { dg-error "" }
-int D::*p8 = static_cast(::x); // { dg-error "" }
+int A::*p7 = static_cast(::x); // { dg-error "ambiguous base" }
+// { dg-message "pointer to member function" "" { target *-*-* } .-1 }
+int D::*p8 = static_cast(::x); // { dg-error "ambiguous base" }
+// { dg-message "pointer to member function" "" { target *-*-* } .-1 }
// Valid conversions which increase cv-qualification.
const int B::*p9 = static_cast(::x);
@@ -35,5 +39,5 @@ int B::*p11 = static_cast(p10); // { dg-
int D::*p12 = static_cast(p9); // { dg-error "casts away
qualifiers" }
// Attempts to change member type.
-float B::*p13 = static_cast(::x); // { dg-error "" }
-float D::*p14 = static_cast(::x); // { dg-error "" }
+float B::*p13 = static_cast(::x); // { dg-error "invalid
.static_cast." }
+float D::*p14 = static_cast(::x); // { dg-error "invalid
.static_cast." }
Index: testsuite/g++.dg/gomp/tpl-atomic-2.C
===
--- testsuite/g++.dg/gomp/tpl-atomic-2.C(revision 277374)
+++ testsuite/g++.dg/gomp/tpl-atomic-2.C(working copy)
@@ -13,7 +13,7 @@ template void f1()
template void f2(float *f)
{
#pragma omp atomic // { dg-error "invalid" }
- *f |= 1; // { dg-error "evaluation" }
+ *f |= 1; // { dg-message "evaluation"
[C++ Patch] Improve build_x_unary_op locations
Hi,
at the moment I'm going through more typeck.c and typeck2.c functions:
the below are some straightforward changes for build_x_unary_op (fwiw,
the resulting locations are also consistent with clang)
Tested x86_64-linux.
Thanks, Paolo.
/cp
2019-10-22 Paolo Carlini
* typeck.c (build_x_unary_op): Use the location_t argument in
three error_at.
/testsuite
2019-10-22 Paolo Carlini
* g++.dg/other/ptrmem8.C: Test locations too.
* g++.dg/template/dtor6.C: Likewise.
Index: cp/typeck.c
===
--- cp/typeck.c (revision 277268)
+++ cp/typeck.c (working copy)
@@ -5829,10 +5831,10 @@ build_x_unary_op (location_t loc, enum tree_code c
if (DECL_CONSTRUCTOR_P (fn) || DECL_DESTRUCTOR_P (fn))
{
if (complain & tf_error)
- error (DECL_CONSTRUCTOR_P (fn)
- ? G_("taking address of constructor %qD")
- : G_("taking address of destructor %qD"),
- fn);
+ error_at (loc, DECL_CONSTRUCTOR_P (fn)
+ ? G_("taking address of constructor %qD")
+ : G_("taking address of destructor %qD"),
+ fn);
return error_mark_node;
}
}
@@ -5847,10 +5849,10 @@ build_x_unary_op (location_t loc, enum tree_code c
{
if (complain & tf_error)
{
- error ("invalid use of %qE to form a "
-"pointer-to-member-function", xarg.get_value ());
+ error_at (loc, "invalid use of %qE to form a "
+ "pointer-to-member-function", xarg.get_value ());
if (TREE_CODE (xarg) != OFFSET_REF)
- inform (input_location, " a qualified-id is required");
+ inform (loc, " a qualified-id is required");
}
return error_mark_node;
}
@@ -5857,9 +5859,9 @@ build_x_unary_op (location_t loc, enum tree_code c
else
{
if (complain & tf_error)
- error ("parentheses around %qE cannot be used to form a"
- " pointer-to-member-function",
- xarg.get_value ());
+ error_at (loc, "parentheses around %qE cannot be used to "
+ "form a pointer-to-member-function",
+ xarg.get_value ());
else
return error_mark_node;
PTRMEM_OK_P (xarg) = 1;
Index: testsuite/g++.dg/other/ptrmem8.C
===
--- testsuite/g++.dg/other/ptrmem8.C(revision 277268)
+++ testsuite/g++.dg/other/ptrmem8.C(working copy)
@@ -6,11 +6,11 @@ struct A {};
template void foo(void (A::* f)())
{
A a;
- &(a.*f); // { dg-error "invalid use of\[^\n\]*\\.\\*\[^\n\]*to
form|qualified-id is required" }
+ &(a.*f); // { dg-error "3:invalid use of\[^\n\]*\\.\\*\[^\n\]*to
form|qualified-id is required" }
}
template void bar(void (A::* f)())
{
A *p;
- &(p->*f);// { dg-error "invalid use of\[^\n\]*->\\*\[^\n\]*to
form|qualified-id is required" }
+ &(p->*f);// { dg-error "3:invalid use of\[^\n\]*->\\*\[^\n\]*to
form|qualified-id is required" }
}
Index: testsuite/g++.dg/template/dtor6.C
===
--- testsuite/g++.dg/template/dtor6.C (revision 277268)
+++ testsuite/g++.dg/template/dtor6.C (working copy)
@@ -5,12 +5,12 @@ template struct A
static int i;
};
-template int A::i = { A::~A }; // { dg-error "non-static member
function" }
+template int A::i = { A::~A }; // { dg-error "36:invalid use of
non-static member function" }
template class A<0>;
struct X { };
-int i1 = X::~X;// { dg-error "non-static member
function" }
-int i2 = ::~X; // { dg-error "address of destructor" }
-int i3 = <0>::~A;// { dg-error "address of destructor" }
+int i1 = X::~X;// { dg-error "13:invalid use of
non-static member function" }
+int i2 = ::~X; // { dg-error "10:taking address of destructor"
}
+int i3 = <0>::~A;// { dg-error "10:taking address of destructor"
}
[C++ Patch] Improve cp_parser_class_head error recovery
Hi,
a few days ago I noticed that for, say, g++.dg/parse/qualified2.C we
were issuing two additional misleading errors after the first one,
mentioning in particular a certain "unnamed class" (I'm reproducing only
the error messages proper):
namespace Glib {
template class Value {};
template <> class Glib::Value {}; // { dg-error "" }
}
qualified2.C:3:29: error: extra qualification not allowed [\-fpermissive\]
qualified2.C:3:46: error: explicit specialization of non-template
‘Glib::’
qualified2.C:3:47: error: abstract declarator ‘Glib::’
used as declaration
Let's see if I can explain clearly enough what I think it's going on.
In cp_parser_class_head, upon the permerror about the extra
qualification, we try to do error recovery, which is particularly
tricky, because we are dealing with a permerror thus we have to make
sure that in case of -fpermissive everything remains internally
consistent anyway. In this context, clearing 'nested_name_specifier' and
'num_templates' doesn't seem a good idea because it does *not* give us
an internal state similar to the one normally obtained when the nested
name specifier is not there, the reason being that, earlier in the
function, when a nested name specifier really isn't there we try
cp_parser_template_id or in case cp_parser_identifier, which set the
locale 'id' and possibly 'template_id' and 'num_templates', whereas
during error recovery we remain so to speak completely empty handed.
Thus, what about not clearing anything? That seems to work at least for
the two testcases below and doesn't cause regressions.
Thanks, Paolo.
/////
/cp
2019-10-18 Paolo Carlini
* parser.c (cp_parser_class_head): Improve error recovery upon
extra qualification error.
/testsuite
2019-10-18 Paolo Carlini
* g++.dg/parse/qualified2.C: Tighten dg-error directive.
* g++.old-deja/g++.other/decl5.C: Don't expect redundant error.
Index: cp/parser.c
===
--- cp/parser.c (revision 277149)
+++ cp/parser.c (working copy)
@@ -24178,12 +24178,8 @@ cp_parser_class_head (cp_parser* parser,
... [or] the definition or explicit instantiation of a
class member of a namespace outside of its namespace. */
if (scope == nested_name_specifier)
- {
- permerror (nested_name_specifier_token_start->location,
-"extra qualification not allowed");
- nested_name_specifier = NULL_TREE;
- num_templates = 0;
- }
+ permerror (nested_name_specifier_token_start->location,
+ "extra qualification not allowed");
}
/* An explicit-specialization must be preceded by "template <>". If
it is not, try to recover gracefully. */
Index: testsuite/g++.dg/parse/qualified2.C
===
--- testsuite/g++.dg/parse/qualified2.C (revision 277144)
+++ testsuite/g++.dg/parse/qualified2.C (working copy)
@@ -1,4 +1,4 @@
namespace Glib {
template class Value {};
- template <> class Glib::Value {}; // { dg-error "" }
+ template <> class Glib::Value {}; // { dg-error "29:extra
qualification" }
}
Index: testsuite/g++.old-deja/g++.other/decl5.C
===
--- testsuite/g++.old-deja/g++.other/decl5.C(revision 277144)
+++ testsuite/g++.old-deja/g++.other/decl5.C(working copy)
@@ -12,7 +12,6 @@ struct A {
int A::m; // { dg-error "extra qualification" }
struct e;
struct A::e {int i;}; // { dg-error "extra qualification" "qual" }
- // { dg-error "anonymous struct" "anon" { target *-*-* } .-1 }
struct A::expand { // { dg-error "qualified name" }
int m;
};
Re: [C++ Patch] Remove most uses of in_system_header_at
.. hi again. We have an issue with this idea which I didn't notice earlier today: there are some libstdc++ *_neg testcases which rely on permerrors being emitted for code in library headers. For example 20_util/ratio/cons/cons_overflow_neg.cc relies on a permerror for the "overflow in constant expression" in constexpr.c. Or, 20_util/variant/visit_neg.cc relies on the permerror for "invalid conversion from .. to .." emitted by convert_like_real. Something seems a little fishy here but I'm not sure which way we want to go: I don't think we want to audit that right here, right now all the permerrors in the front-end which could potentially be involved in this kind of issue. Even if we, say, promote to plain errors the above two, that seems brittle, we got many permerrors which in principle should be real errors. Paolo.
Re: [C++ Patch] Remove most uses of in_system_header_at
Hi,
On 17/10/19 05:15, Jason Merrill wrote:
On 10/16/19 11:59 AM, Paolo Carlini wrote:
... the below, slightly extended patch: 1- Makes sure the
in_system_header_at calls surviving in decl.c get the same location
used for the corresponding diagnostic
Hmm, we probably want to change permerror to respect
warn_system_headers like warning and pedwarn.
I see. Certainly it enables cleaning up those two remaining usages in
decl.c.
Which is the right place to implement this? The beginning of
diagnostic_impl figures out if a given permerror boils down to an actual
error or a warning: if we use in_system_header_at at that level, part of
the permissive_error_kind macro, then the warning is handled as any
other other warning, thus by default suppressed in system headers, etc.
Makes sense? Tested x86_64-linux.
Thanks, Paolo.
Index: cp/decl.c
===
--- cp/decl.c (revision 277097)
+++ cp/decl.c (working copy)
@@ -4933,10 +4933,9 @@ check_tag_decl (cp_decl_specifier_seq *declspecs,
"multiple types in one declaration");
else if (declspecs->redefined_builtin_type)
{
- if (!in_system_header_at (input_location))
- permerror (declspecs->locations[ds_redefined_builtin_type_spec],
- "redeclaration of C++ built-in type %qT",
- declspecs->redefined_builtin_type);
+ permerror (declspecs->locations[ds_redefined_builtin_type_spec],
+"redeclaration of C++ built-in type %qT",
+declspecs->redefined_builtin_type);
return NULL_TREE;
}
@@ -4984,7 +4983,8 @@ check_tag_decl (cp_decl_specifier_seq *declspecs,
--end example] */
if (saw_typedef)
{
- error ("missing type-name in typedef-declaration");
+ error_at (declspecs->locations[ds_typedef],
+ "missing type-name in typedef-declaration");
return NULL_TREE;
}
/* Anonymous unions are objects, so they can have specifiers. */;
@@ -9328,7 +9328,6 @@ grokfndecl (tree ctype,
}
/* 17.6.3.3.5 */
if (suffix[0] != '_'
- && !in_system_header_at (location)
&& !current_function_decl && !(friendp && !funcdef_flag))
warning_at (location, OPT_Wliteral_suffix,
"literal operator suffixes not preceded by %<_%>"
@@ -10036,8 +10035,6 @@ compute_array_index_type_loc (location_t name_loc,
indicated by the state of complain), so that
another substitution can be found. */
return error_mark_node;
- else if (in_system_header_at (input_location))
- /* Allow them in system headers because glibc uses them. */;
else if (name)
pedwarn (loc, OPT_Wpedantic,
"ISO C++ forbids zero-size array %qD", name);
@@ -11004,7 +11001,7 @@ grokdeclarator (const cp_declarator *declarator,
if (type_was_error_mark_node)
/* We've already issued an error, don't complain more. */;
- else if (in_system_header_at (input_location) || flag_ms_extensions)
+ else if (flag_ms_extensions)
/* Allow it, sigh. */;
else if (! is_main)
permerror (id_loc, "ISO C++ forbids declaration of %qs with no type",
@@ -11037,7 +11034,7 @@ grokdeclarator (const cp_declarator *declarator,
}
/* Don't pedwarn if the alternate "__intN__" form has been used instead
of "__intN". */
- else if (!int_n_alt && pedantic && ! in_system_header_at
(input_location))
+ else if (!int_n_alt && pedantic)
pedwarn (declspecs->locations[ds_type_spec], OPT_Wpedantic,
"ISO C++ does not support %<__int%d%> for %qs",
int_n_data[declspecs->int_n_idx].bitsize, name);
@@ -12695,10 +12692,7 @@ grokdeclarator (const cp_declarator *declarator,
else
{
/* Array is a flexible member. */
- if (in_system_header_at (input_location))
- /* Do not warn on flexible array members in system
-headers because glibc uses them. */;
- else if (name)
+ if (name)
pedwarn (id_loc, OPT_Wpedantic,
"ISO C++ forbids flexible array member %qs", name);
else
Index: cp/error.c
===
--- cp/error.c (revision 277097)
+++ cp/error.c (working copy)
@@ -4317,10 +4317,7 @@ cp_printer (pretty_printer *pp, text_info *text, c
void
maybe_warn_cpp0x (cpp0x_warn_str str)
{
- if ((cxx_dialect == cxx98) && !in_system_header_at (input
Re: [C++ Patch] Remove most uses of in_system_header_at
... the below, slightly extended patch: 1- Makes sure the
in_system_header_at calls surviving in decl.c get the same location used
for the corresponding diagnostic; exploit locations[ds_typedef] in an
error_at in grokdeclarator.
Tested, as usual, on x86_64-linux.
Thanks, Paolo.
/
/cp
2019-10-16 Paolo Carlini
* decl.c (grokfndecl): Remove redundant use of in_system_header_at.
(compute_array_index_type_loc): Likewise.
(grokdeclarator): Likewise.
* error.c (cp_printer): Likewise.
* lambda.c (add_default_capture): Likewise.
* parser.c (cp_parser_primary_expression): Likewise.
(cp_parser_selection_statement): Likewise.
(cp_parser_toplevel_declaration): Likewise.
(cp_parser_enumerator_list): Likewise.
(cp_parser_using_declaration): Likewise.
(cp_parser_member_declaration): Likewise.
(cp_parser_exception_specification_opt): Likewise.
(cp_parser_std_attribute_spec): Likewise.
* pt.c (do_decl_instantiation): Likewise.
(do_type_instantiation): Likewise.
* typeck.c (cp_build_unary_op): Likewise.
* decl.c (check_tag_decl): Pass to in_system_header_at the same
location used for the permerror.
(grokdeclarator): Likewise.
* decl.c (check_tag_decl): Use locations[ds_typedef] in error_at.
/testsuite
2019-10-16 Paolo Carlini
* g++.old-deja/g++.other/decl9.C: Check locations too.
Index: cp/decl.c
===
--- cp/decl.c (revision 276984)
+++ cp/decl.c (working copy)
@@ -4933,9 +4933,9 @@ check_tag_decl (cp_decl_specifier_seq *declspecs,
"multiple types in one declaration");
else if (declspecs->redefined_builtin_type)
{
- if (!in_system_header_at (input_location))
- permerror (declspecs->locations[ds_redefined_builtin_type_spec],
- "redeclaration of C++ built-in type %qT",
+ location_t loc = declspecs->locations[ds_redefined_builtin_type_spec];
+ if (!in_system_header_at (loc))
+ permerror (loc, "redeclaration of C++ built-in type %qT",
declspecs->redefined_builtin_type);
return NULL_TREE;
}
@@ -4984,7 +4984,8 @@ check_tag_decl (cp_decl_specifier_seq *declspecs,
--end example] */
if (saw_typedef)
{
- error ("missing type-name in typedef-declaration");
+ error_at (declspecs->locations[ds_typedef],
+ "missing type-name in typedef-declaration");
return NULL_TREE;
}
/* Anonymous unions are objects, so they can have specifiers. */;
@@ -9328,7 +9329,6 @@ grokfndecl (tree ctype,
}
/* 17.6.3.3.5 */
if (suffix[0] != '_'
- && !in_system_header_at (location)
&& !current_function_decl && !(friendp && !funcdef_flag))
warning_at (location, OPT_Wliteral_suffix,
"literal operator suffixes not preceded by %<_%>"
@@ -10036,8 +10036,6 @@ compute_array_index_type_loc (location_t name_loc,
indicated by the state of complain), so that
another substitution can be found. */
return error_mark_node;
- else if (in_system_header_at (input_location))
- /* Allow them in system headers because glibc uses them. */;
else if (name)
pedwarn (loc, OPT_Wpedantic,
"ISO C++ forbids zero-size array %qD", name);
@@ -11004,7 +11002,7 @@ grokdeclarator (const cp_declarator *declarator,
if (type_was_error_mark_node)
/* We've already issued an error, don't complain more. */;
- else if (in_system_header_at (input_location) || flag_ms_extensions)
+ else if (in_system_header_at (id_loc) || flag_ms_extensions)
/* Allow it, sigh. */;
else if (! is_main)
permerror (id_loc, "ISO C++ forbids declaration of %qs with no type",
@@ -11037,7 +11035,7 @@ grokdeclarator (const cp_declarator *declarator,
}
/* Don't pedwarn if the alternate "__intN__" form has been used instead
of "__intN". */
- else if (!int_n_alt && pedantic && ! in_system_header_at
(input_location))
+ else if (!int_n_alt && pedantic)
pedwarn (declspecs->locations[ds_type_spec], OPT_Wpedantic,
"ISO C++ does not support %<__int%d%> for %qs",
int_n_data[declspecs->int_n_idx].bitsize, name);
@@ -12695,10 +12693,7 @@ grokdeclarator (const cp_declarator *declarator,
else
{
/* Array is a flexible member. */
- if (in_system_header_at (input_location))
- /* Do not warn o
Re: [C++ Patch] Remove most uses of in_system_header_at
Hi, On 12/10/19 00:28, Jakub Jelinek wrote: @@ -10036,8 +10035,6 @@ compute_array_index_type_loc (location_t name_loc, indicated by the state of complain), so that another substitution can be found. */ return error_mark_node; - else if (in_system_header_at (input_location)) - /* Allow them in system headers because glibc uses them. */; else if (name) pedwarn (loc, OPT_Wpedantic, "ISO C++ forbids zero-size array %qD", name); But this one is unclear, in_system_header_at is testing a different location from what will be used by pedwarn, so if input_location is in system header and loc is not, it didn't pedwarn before and now it will. Similarly various other spots in the patch. I haven't tried to check in detail what exactly we want in each case, all I want to say is that some cases are obvious, other cases are not. Thanks for noticiing. We already discussed that a bit with Marek: we believe it should not make a difference, in practice, because the difference should only be quite small and if the first location is in the header the second one is supposed to be there too. More importantly, as far as I know, the difference is just an historical artifact and many actually I created myself ;) when I started changing decl.c, decl2.c, etc, to pass locations more consistently: I simply left the in_system_header bits alone for simplicity (as discussed with Marek). Thanks, Paolo.
[C++ Patch] Remove most uses of in_system_header_at
Hi,
as promised, this removes most uses of the predicate. I left alone the
two occurrences in decl.c which guard permerrors, not warnings, and two
more in parser.c which should be a (minor) optimization (particularly
that in cp_parser_cast_expression).
Tested x86_64-linux.
Thanks, Paolo.
///
2019-10-11 Paolo Carlini
* decl.c (grokfndecl): Remove redundant use of in_system_header_at.
(compute_array_index_type_loc): Likewise.
(grokdeclarator): Likewise.
* error.c (cp_printer): Likewise.
* lambda.c (add_default_capture): Likewise.
* parser.c (cp_parser_primary_expression): Likewise.
(cp_parser_selection_statement): Likewise.
(cp_parser_toplevel_declaration): Likewise.
(cp_parser_enumerator_list): Likewise.
(cp_parser_using_declaration): Likewise.
(cp_parser_member_declaration): Likewise.
(cp_parser_exception_specification_opt): Likewise.
(cp_parser_std_attribute_spec): Likewise.
* pt.c (do_decl_instantiation): Likewise.
(do_type_instantiation): Likewise.
* typeck.c (cp_build_unary_op): Likewise.
Index: decl.c
===
--- decl.c (revision 276903)
+++ decl.c (working copy)
@@ -9328,7 +9328,6 @@ grokfndecl (tree ctype,
}
/* 17.6.3.3.5 */
if (suffix[0] != '_'
- && !in_system_header_at (location)
&& !current_function_decl && !(friendp && !funcdef_flag))
warning_at (location, OPT_Wliteral_suffix,
"literal operator suffixes not preceded by %<_%>"
@@ -10036,8 +10035,6 @@ compute_array_index_type_loc (location_t name_loc,
indicated by the state of complain), so that
another substitution can be found. */
return error_mark_node;
- else if (in_system_header_at (input_location))
- /* Allow them in system headers because glibc uses them. */;
else if (name)
pedwarn (loc, OPT_Wpedantic,
"ISO C++ forbids zero-size array %qD", name);
@@ -11037,7 +11034,7 @@ grokdeclarator (const cp_declarator *declarator,
}
/* Don't pedwarn if the alternate "__intN__" form has been used instead
of "__intN". */
- else if (!int_n_alt && pedantic && ! in_system_header_at
(input_location))
+ else if (!int_n_alt && pedantic)
pedwarn (declspecs->locations[ds_type_spec], OPT_Wpedantic,
"ISO C++ does not support %<__int%d%> for %qs",
int_n_data[declspecs->int_n_idx].bitsize, name);
@@ -12695,10 +12692,7 @@ grokdeclarator (const cp_declarator *declarator,
else
{
/* Array is a flexible member. */
- if (in_system_header_at (input_location))
- /* Do not warn on flexible array members in system
-headers because glibc uses them. */;
- else if (name)
+ if (name)
pedwarn (id_loc, OPT_Wpedantic,
"ISO C++ forbids flexible array member %qs", name);
else
Index: error.c
===
--- error.c (revision 276903)
+++ error.c (working copy)
@@ -4317,10 +4317,7 @@ cp_printer (pretty_printer *pp, text_info *text, c
void
maybe_warn_cpp0x (cpp0x_warn_str str)
{
- if ((cxx_dialect == cxx98) && !in_system_header_at (input_location))
-/* We really want to suppress this warning in system headers,
- because libstdc++ uses variadic templates even when we aren't
- in C++0x mode. */
+ if (cxx_dialect == cxx98)
switch (str)
{
case CPP0X_INITIALIZER_LISTS:
Index: lambda.c
===
--- lambda.c(revision 276903)
+++ lambda.c(working copy)
@@ -697,8 +697,7 @@ add_default_capture (tree lambda_stack, tree id, t
/* Warn about deprecated implicit capture of this via [=]. */
if (cxx_dialect >= cxx2a
&& this_capture_p
- && LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda) == CPLD_COPY
- && !in_system_header_at (LAMBDA_EXPR_LOCATION (lambda)))
+ && LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda) == CPLD_COPY)
{
if (warning_at (LAMBDA_EXPR_LOCATION (lambda), OPT_Wdeprecated,
"implicit capture of %qE via %<[=]%> is deprecated "
Index: parser.c
===
--- parser.c(revision 276903)
+++ parser.c(working copy)
@@ -5364,8 +5364,7 @@ cp_parser_primary_expression (cp_parser *parser,
{
Re: [C++ Patch] Remove RROTATE_EXPR and LROTATE_EXPR handling
Hi,
On 11/10/19 18:25, Jason Merrill wrote:
On 10/10/19 2:33 PM, Paolo Carlini wrote:
Hi,
On 10/10/19 19:57, Jason Merrill wrote:
On 10/10/19 1:53 PM, Jakub Jelinek wrote:
On Thu, Oct 10, 2019 at 06:12:02PM +0200, Paolo Carlini wrote:
while working on cp_build_binary_op I noticed that the testsuite
wasn't
exercising the warnings in case RROTATE_EXPR / LROTATE_EXPR, even
more the
code handling those tree codes seemed completely unused. Turned
out that the
C front-end doesn't handle those tree codes at all: I'm coming to the
conclusion that the C++ front-end bits too are now obsolete and
may be
removed, because only the middle-end generates those codes in
order to
implement optimizations. Anything I'm missing? Any additional
testing?
I guess it depends on where.
fold_binary_loc certainly has code to create {L,R}ROTATE_EXPR,
just look at
unsigned foo (unsigned x)
{
return (x << 3) + (x >> (__SIZEOF_INT__ * __CHAR_BIT__ - 3));
}
unsigned bar (unsigned x, unsigned y)
{
return (x << y) | (x >> (__SIZEOF_INT__ * __CHAR_BIT__ - y));
}
and the *.original dump.
The cp_build_binary_op case is unlikely to ever trigger, unless
we'd rerun
it on cp_folded trees.
cxx_eval_constant_expression is unlikely, because recently we've
switched to
performing constexpr evaluation on pre-cp_folded bodies, not sure
if we
never encounter folded trees though.
cp_fold itself depends on whether we ever reprocess the already folded
trees, I'd be afraid we could.
True, and the failure mode there is silent. Let's leave the codes
in that switch.
Ok, thanks Jason and Jakub for the additional information.
While I give this more thought and maybe manage to come up with a
testcase triggering the warning, shall we simply pass the location to
those two warnings too - cannot hurt, AFAICS?
I meant let's omit the changes to cp_fold, the rest of the patch is
still OK.
Nice, thanks, I'll do that.
Paolo.
Re: [C++ Patch] One more DECL_SOURCE_LOCATION
Hi, On 11/10/19 17:52, Jakub Jelinek wrote: On Fri, Oct 11, 2019 at 05:34:16PM +0200, Paolo Carlini wrote: Hi again... On 11/10/19 17:30, Paolo Carlini wrote: Oh nice, I wasn't aware of that, to be honest, probably we should audit the front-end for more such redundant uses. ... and I can confirm that we have *many*. If we agree that removing all of them is the way to go I can do that in a follow up patch. If they just guard a pedwarn/warning/warning_at etc., that's fine, if they guard also some code that needs to compute something for the diagnostics, it might not be redundant. Indeed. We got many of the very straightforward ones ;) Paolo.
Re: [C++ Patch] One more DECL_SOURCE_LOCATION
Hi again... On 11/10/19 17:30, Paolo Carlini wrote: Oh nice, I wasn't aware of that, to be honest, probably we should audit the front-end for more such redundant uses. ... and I can confirm that we have *many*. If we agree that removing all of them is the way to go I can do that in a follow up patch. Thanks, Paolo.
Re: [C++ Patch] One more DECL_SOURCE_LOCATION
Hi, On 11/10/19 17:23, Marek Polacek wrote: I mean the latter; pedwarn will check for diagnostic_report_warnings_p so the pedwarn will not trigger in a system header unless -Wsystem-headers even without that check. Oh nice, I wasn't aware of that, to be honest, probably we should audit the front-end for more such redundant uses. Anyway, consider it dropped in this case, I'm re-running the testsuite to be safe. Thanks, Paolo.
Re: [C++ Patch] One more DECL_SOURCE_LOCATION
Hi, On 11/10/19 15:37, Marek Polacek wrote: Index: cp/decl.c === --- cp/decl.c (revision 276845) +++ cp/decl.c (working copy) @@ -4992,7 +4992,8 @@ check_tag_decl (cp_decl_specifier_seq *declspecs, if (TREE_CODE (declared_type) != UNION_TYPE && !in_system_header_at (input_location)) - pedwarn (input_location, OPT_Wpedantic, "ISO C++ prohibits anonymous structs"); + pedwarn (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (declared_type)), +OPT_Wpedantic, "ISO C++ prohibits anonymous structs"); The change looks fine but the in_system_header_at line can be dropped, right? What do you mean, exactly? Do you mean that, more correctly, we should use DECL_SOURCE_LOCATION for it too or that in fact is and was already completely redundant? I agree with the former, I didn't bother changing it (likely in a couple of other places too) because in practice input_location should work fine anyway as far as noticing that we are in system header is concerned and is simpler. If you mean the latter, I'm not sure, I don't really see why... Paolo.
[C++ Patch] One more DECL_SOURCE_LOCATION
Hi,
another straightforward DECL_SOURCE_LOCATION (TYPE_MAIN_DECL consistent
with the one I used in build_anon_union_vars. Tested x86_64-linux.
Thanks, Paolo.
/
/cp
2019-10-11 Paolo Carlini
* decl.c (check_tag_decl): Use DECL_SOURCE_LOCATION.
/testsuite
2019-10-11 Paolo Carlini
* g++.dg/cpp0x/constexpr-union5.C: Test location(s) too.
* g++.dg/diagnostic/bitfld2.C: Likewise.
* g++.dg/ext/anon-struct1.C: Likewise.
* g++.dg/ext/anon-struct6.C: Likewise.
* g++.dg/ext/flexary19.C: Likewise.
* g++.dg/ext/flexary9.C: Likewise.
* g++.dg/template/error17.C: Likewise.
Index: cp/decl.c
===
--- cp/decl.c (revision 276845)
+++ cp/decl.c (working copy)
@@ -4992,7 +4992,8 @@ check_tag_decl (cp_decl_specifier_seq *declspecs,
if (TREE_CODE (declared_type) != UNION_TYPE
&& !in_system_header_at (input_location))
- pedwarn (input_location, OPT_Wpedantic, "ISO C++ prohibits anonymous
structs");
+ pedwarn (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (declared_type)),
+OPT_Wpedantic, "ISO C++ prohibits anonymous structs");
}
else
Index: testsuite/g++.dg/cpp0x/constexpr-union5.C
===
--- testsuite/g++.dg/cpp0x/constexpr-union5.C (revision 276845)
+++ testsuite/g++.dg/cpp0x/constexpr-union5.C (working copy)
@@ -23,16 +23,16 @@ SA((a.i == 42));
struct B
{
- struct {
+ struct { // { dg-warning "10:ISO C\\+\\+ prohibits
anonymous struct" }
int h;
-struct {
+struct { // { dg-warning "12:ISO C\\+\\+ prohibits
anonymous struct" }
union {
unsigned char i;
int j;
};
int k;
-}; // { dg-warning "anonymous struct" }
- }; // { dg-warning "anonymous struct" }
+};
+ };
int l;
constexpr B(): h(1), i(2), k(3), l(4) {}
Index: testsuite/g++.dg/diagnostic/bitfld2.C
===
--- testsuite/g++.dg/diagnostic/bitfld2.C (revision 276845)
+++ testsuite/g++.dg/diagnostic/bitfld2.C (working copy)
@@ -6,4 +6,4 @@ template struct A
struct {} : 2; // { dg-error "expected ';' after struct" "expected" }
};
// { dg-error "ISO C.. forbids declaration" "declaration" { target *-*-* } 6 }
-// { dg-error "ISO C.. prohibits anonymous" "anonymous" { target *-*-* } 6 }
+// { dg-error "10:ISO C.. prohibits anonymous" "anonymous" { target *-*-* } 6 }
Index: testsuite/g++.dg/ext/anon-struct1.C
===
--- testsuite/g++.dg/ext/anon-struct1.C (revision 276845)
+++ testsuite/g++.dg/ext/anon-struct1.C (working copy)
@@ -19,7 +19,7 @@ char testD[sizeof(C::D) == sizeof(A) ? 1 : -1];
/* GNU extension. */
struct E {
- struct { char z; }; /* { dg-error "prohibits anonymous structs" } */
+ struct { char z; }; /* { dg-error "10:ISO C\\+\\+ prohibits
anonymous structs" } */
char e;
};
@@ -45,6 +45,6 @@ char testH[sizeof(H) == 2 * sizeof(A) ? 1 : -1];
/* Make sure __extension__ gets turned back off. */
struct I {
- struct { char z; }; /* { dg-error "prohibits anonymous structs" } */
+ struct { char z; }; /* { dg-error "10:ISO C\\+\\+ prohibits
anonymous structs" } */
char i;
};
Index: testsuite/g++.dg/ext/anon-struct6.C
===
--- testsuite/g++.dg/ext/anon-struct6.C (revision 276845)
+++ testsuite/g++.dg/ext/anon-struct6.C (working copy)
@@ -3,8 +3,8 @@
struct A
{
struct
- {
+ { // { dg-error "3:ISO C\\+\\+ prohibits anonymous structs" }
struct { static int i; }; // { dg-error "prohibits anonymous
structs|non-static data members|unnamed class" }
void foo() { i; } // { dg-error "public non-static data" }
- }; // { dg-error "prohibits anonymous structs" }
+ };
};
Index: testsuite/g++.dg/ext/flexary19.C
===
--- testsuite/g++.dg/ext/flexary19.C(revision 276845)
+++ testsuite/g++.dg/ext/flexary19.C(working copy)
@@ -146,13 +146,13 @@ struct S16
{
int i;
- struct { // { dg-warning "invalid use" }
+ struct { // { dg-warning "10:ISO C\\+\\+ prohibits anonymous
struct|invalid use" }
// A flexible array as a sole member of an anonymous struct is
// rejected with an error in C mode but emits just a pedantic
// warning in C++. Other than excessive pedantry there is no
// reason to reject it.
int a[];
- };
Re: [C++ Patch] Remove RROTATE_EXPR and LROTATE_EXPR handling
Hi,
On 10/10/19 19:57, Jason Merrill wrote:
On 10/10/19 1:53 PM, Jakub Jelinek wrote:
On Thu, Oct 10, 2019 at 06:12:02PM +0200, Paolo Carlini wrote:
while working on cp_build_binary_op I noticed that the testsuite wasn't
exercising the warnings in case RROTATE_EXPR / LROTATE_EXPR, even
more the
code handling those tree codes seemed completely unused. Turned out
that the
C front-end doesn't handle those tree codes at all: I'm coming to the
conclusion that the C++ front-end bits too are now obsolete and may be
removed, because only the middle-end generates those codes in order to
implement optimizations. Anything I'm missing? Any additional testing?
I guess it depends on where.
fold_binary_loc certainly has code to create {L,R}ROTATE_EXPR,
just look at
unsigned foo (unsigned x)
{
return (x << 3) + (x >> (__SIZEOF_INT__ * __CHAR_BIT__ - 3));
}
unsigned bar (unsigned x, unsigned y)
{
return (x << y) | (x >> (__SIZEOF_INT__ * __CHAR_BIT__ - y));
}
and the *.original dump.
The cp_build_binary_op case is unlikely to ever trigger, unless we'd
rerun
it on cp_folded trees.
cxx_eval_constant_expression is unlikely, because recently we've
switched to
performing constexpr evaluation on pre-cp_folded bodies, not sure if we
never encounter folded trees though.
cp_fold itself depends on whether we ever reprocess the already folded
trees, I'd be afraid we could.
True, and the failure mode there is silent. Let's leave the codes in
that switch.
Ok, thanks Jason and Jakub for the additional information.
While I give this more thought and maybe manage to come up with a
testcase triggering the warning, shall we simply pass the location to
those two warnings too - cannot hurt, AFAICS?
Thanks, Paolo.
Index: typeck.c
===
--- typeck.c(revision 276845)
+++ typeck.c(working copy)
@@ -4906,16 +4906,16 @@ cp_build_binary_op (const op_location_t ,
if (tree_int_cst_lt (op1, integer_zero_node))
{
if (complain & tf_warning)
- warning (0, (code == LROTATE_EXPR)
- ? G_("left rotate count is negative")
- : G_("right rotate count is negative"));
+ warning_at (location, 0, (code == LROTATE_EXPR)
+ ? G_("left rotate count is negative")
+ : G_("right rotate count is negative"));
}
else if (compare_tree_int (op1, TYPE_PRECISION (type0)) >= 0)
{
if (complain & tf_warning)
- warning (0, (code == LROTATE_EXPR)
- ? G_("left rotate count >= width of type")
- : G_("right rotate count >= width of type"));
+ warning_at (location, 0, (code == LROTATE_EXPR)
+ ? G_("left rotate count >= width of type")
+ : G_("right rotate count >= width of type"));
}
}
/* Convert the shift-count to an integer, regardless of
[C++ Patch] Remove RROTATE_EXPR and LROTATE_EXPR handling
Hi,
while working on cp_build_binary_op I noticed that the testsuite wasn't
exercising the warnings in case RROTATE_EXPR / LROTATE_EXPR, even more
the code handling those tree codes seemed completely unused. Turned out
that the C front-end doesn't handle those tree codes at all: I'm coming
to the conclusion that the C++ front-end bits too are now obsolete and
may be removed, because only the middle-end generates those codes in
order to implement optimizations. Anything I'm missing? Any additional
testing? Tested x86_64-linux.
Thanks, Paolo.
///
2019-10-10 Paolo Carlini
* typeck.c (cp_build_binary_op): Do not handle RROTATE_EXPR and
LROTATE_EXPR.
* constexpr.c (cxx_eval_constant_expression): Likewise.
(potential_constant_expression_1): Likewise.
* cp-gimplify.c (cp_fold): Likewise.
* pt.c (tsubst_copy): Likewise.
Index: constexpr.c
===
--- constexpr.c (revision 276805)
+++ constexpr.c (working copy)
@@ -5115,8 +5115,6 @@ cxx_eval_constant_expression (const constexpr_ctx
case MAX_EXPR:
case LSHIFT_EXPR:
case RSHIFT_EXPR:
-case LROTATE_EXPR:
-case RROTATE_EXPR:
case BIT_IOR_EXPR:
case BIT_XOR_EXPR:
case BIT_AND_EXPR:
@@ -7103,8 +7101,6 @@ potential_constant_expression_1 (tree t, bool want
case MAX_EXPR:
case LSHIFT_EXPR:
case RSHIFT_EXPR:
-case LROTATE_EXPR:
-case RROTATE_EXPR:
case BIT_IOR_EXPR:
case BIT_XOR_EXPR:
case BIT_AND_EXPR:
Index: cp-gimplify.c
===
--- cp-gimplify.c (revision 276805)
+++ cp-gimplify.c (working copy)
@@ -2468,8 +2468,6 @@ cp_fold (tree x)
case MAX_EXPR:
case LSHIFT_EXPR:
case RSHIFT_EXPR:
-case LROTATE_EXPR:
-case RROTATE_EXPR:
case BIT_AND_EXPR:
case BIT_IOR_EXPR:
case BIT_XOR_EXPR:
Index: pt.c
===
--- pt.c(revision 276805)
+++ pt.c(working copy)
@@ -16308,8 +16308,6 @@ tsubst_copy (tree t, tree args, tsubst_flags_t com
case TRUTH_OR_EXPR:
case RSHIFT_EXPR:
case LSHIFT_EXPR:
-case RROTATE_EXPR:
-case LROTATE_EXPR:
case EQ_EXPR:
case NE_EXPR:
case MAX_EXPR:
@@ -18913,8 +18911,6 @@ tsubst_copy_and_build (tree t,
case TRUTH_OR_EXPR:
case RSHIFT_EXPR:
case LSHIFT_EXPR:
-case RROTATE_EXPR:
-case LROTATE_EXPR:
case EQ_EXPR:
case NE_EXPR:
case MAX_EXPR:
Index: typeck.c
===
--- typeck.c(revision 276805)
+++ typeck.c(working copy)
@@ -4896,35 +4896,6 @@ cp_build_binary_op (const op_location_t ,
}
break;
-case RROTATE_EXPR:
-case LROTATE_EXPR:
- if (code0 == INTEGER_TYPE && code1 == INTEGER_TYPE)
- {
- result_type = type0;
- if (TREE_CODE (op1) == INTEGER_CST)
- {
- if (tree_int_cst_lt (op1, integer_zero_node))
- {
- if (complain & tf_warning)
- warning (0, (code == LROTATE_EXPR)
- ? G_("left rotate count is negative")
- : G_("right rotate count is negative"));
- }
- else if (compare_tree_int (op1, TYPE_PRECISION (type0)) >= 0)
- {
- if (complain & tf_warning)
- warning (0, (code == LROTATE_EXPR)
- ? G_("left rotate count >= width of type")
- : G_("right rotate count >= width of type"));
- }
- }
- /* Convert the shift-count to an integer, regardless of
-size of value being shifted. */
- if (TYPE_MAIN_VARIANT (TREE_TYPE (op1)) != integer_type_node)
- op1 = cp_convert (integer_type_node, op1, complain);
- }
- break;
-
case EQ_EXPR:
case NE_EXPR:
if (code0 == VECTOR_TYPE && code1 == VECTOR_TYPE)
[C++ Patch] Fix cp_build_binary_op locations
Hi,
this largely mechanical patch fixes those errors in cp_build_binary_op
which weren't exploiting yet the op_location_t argument, thus aligning
the locations to those of the C front-end. Plus an additional use for
DECL_SOURCE_LOCATION in decl.c. Tested x86_64-linux.
Thanks, Paolo.
///
/cp
2019-10-09 Paolo Carlini
* decl.c (grok_ctor_properties): Use DECL_SOURCE_LOCATION.
* typeck.c (cp_build_binary_op): Use the op_location_t argument
in many error messages.
/testsuite
2019-10-09 Paolo Carlini
* c-c++-common/Waddress-1.c: Test locations too.
* c-c++-common/Wpointer-compare-1.c: Likewise.
* c-c++-common/Wshift-count-negative-1.c: Likewise.
* c-c++-common/Wshift-count-overflow-1.c: Likewise.
* c-c++-common/Wshift-negative-value-1.c: Likewise.
* c-c++-common/Wshift-negative-value-2.c: Likewise.
* c-c++-common/Wshift-negative-value-5.c: Likewise.
* c-c++-common/pr48418.c: Likewise.
* c-c++-common/pr65830.c: Likewise.
* c-c++-common/pr69764.c: Likewise.
* g++.dg/cpp0x/constexpr-array-ptr10.C: Likewise.
* g++.dg/cpp0x/nullptr37.C: Likewise.
* g++.dg/template/crash126.C: Likewise.
* g++.dg/template/crash129.C: Likewise.
* g++.dg/warn/Wextra-3.C: Likewise.
* g++.dg/warn/Wfloat-equal-1.C: Likewise.
* g++.dg/warn/Wstring-literal-comparison-1.C: Likewise.
* g++.dg/warn/Wstring-literal-comparison-2.C: Likewise.
* g++.dg/warn/pointer-integer-comparison.C: Likewise.
* g++.old-deja/g++.jason/crash8.C: Likewise.
Index: cp/decl.c
===
--- cp/decl.c (revision 276756)
+++ cp/decl.c (working copy)
@@ -13738,7 +13738,8 @@ grok_ctor_properties (const_tree ctype, const_tree
or implicitly defined), there's no need to worry about their
existence. Theoretically, they should never even be
instantiated, but that's hard to forestall. */
- error ("invalid constructor; you probably meant %<%T (const %T&)%>",
+ error_at (DECL_SOURCE_LOCATION (decl),
+ "invalid constructor; you probably meant %<%T (const %T&)%>",
ctype, ctype);
return false;
}
Index: cp/typeck.c
===
--- cp/typeck.c (revision 276756)
+++ cp/typeck.c (working copy)
@@ -4475,7 +4475,8 @@ cp_build_binary_op (const op_location_t ,
if (t != error_mark_node)
{
if (complain & tf_error)
- permerror (input_location, "assuming cast to type %qT from
overloaded function",
+ permerror (location,
+ "assuming cast to type %qT from overloaded function",
TREE_TYPE (t));
op0 = t;
}
@@ -4486,7 +4487,8 @@ cp_build_binary_op (const op_location_t ,
if (t != error_mark_node)
{
if (complain & tf_error)
- permerror (input_location, "assuming cast to type %qT from
overloaded function",
+ permerror (location,
+ "assuming cast to type %qT from overloaded function",
TREE_TYPE (t));
op1 = t;
}
@@ -4809,8 +4811,8 @@ cp_build_binary_op (const op_location_t ,
{
if ((complain & tf_warning)
&& c_inhibit_evaluation_warnings == 0)
- warning (OPT_Wshift_count_negative,
-"right shift count is negative");
+ warning_at (location, OPT_Wshift_count_negative,
+ "right shift count is negative");
}
else
{
@@ -4817,8 +4819,8 @@ cp_build_binary_op (const op_location_t ,
if (compare_tree_int (const_op1, TYPE_PRECISION (type0)) >= 0
&& (complain & tf_warning)
&& c_inhibit_evaluation_warnings == 0)
- warning (OPT_Wshift_count_overflow,
-"right shift count >= width of type");
+ warning_at (location, OPT_Wshift_count_overflow,
+ "right shift count >= width of type");
}
}
/* Avoid converting op1 to result_type later. */
@@ -4856,8 +4858,8 @@ cp_build_binary_op (const op_location_t ,
&& tree_int_cst_sgn (const_op0) < 0
&& (complain & tf_warning)
&& c_inhibit_evaluation_warnings == 0)
- warning (OPT_Wshift_negative_value,
-"left shift of negative value");
+ warning_at (location, OPT_Wshift_negative_valu
[C++ Patch] Fix cp_parser_delete_expression and cp_parser_throw_expression locations and more
Hi,
the below fixes those two functions consistently with
cp_parser_new_expression. Additionally, a few rather straightforward
tweaks along the usual lines, more DECL_SOURCE_LOCATION and id_loc.
Tested x86_64-linux.
Thanks, Paolo.
/
/cp
2019-10-07 Paolo Carlini
* call.c (resolve_args): Use cp_expr_loc_or_input_loc in one place.
* decl.c (grokdeclarator): Use id_loc in one place.
* decl2.c (build_anon_union_vars): Use DECL_SOURCE_LOCATION.
* parser.c (cp_parser_delete_expression): Fix the location of the
returned expression.
(cp_parser_throw_expression): Likewise.
* pt.c (determine_specialization): Use DECL_SOURCE_LOCATION.
/testsuite
2019-10-07 Paolo Carlini
* g++.dg/diagnostic/not-a-function-template-1.C: New.
* g++.dg/template/crash107.C: Adjust expected location.
* g++.dg/template/dependent-expr1.C: Check locations.
* g++.dg/template/error17.C: Check location.
Index: cp/call.c
===
--- cp/call.c (revision 276646)
+++ cp/call.c (working copy)
@@ -4381,7 +4381,8 @@ resolve_args (vec *args, tsubst_flags
else if (VOID_TYPE_P (TREE_TYPE (arg)))
{
if (complain & tf_error)
- error ("invalid use of void expression");
+ error_at (cp_expr_loc_or_input_loc (arg),
+ "invalid use of void expression");
return NULL;
}
else if (invalid_nonstatic_memfn_p (EXPR_LOCATION (arg), arg, complain))
Index: cp/decl.c
===
--- cp/decl.c (revision 276646)
+++ cp/decl.c (working copy)
@@ -12754,8 +12754,8 @@ grokdeclarator (const cp_declarator *declarator,
tree tmpl = TREE_OPERAND (unqualified_id, 0);
if (variable_template_p (tmpl))
{
- error ("specialization of variable template %qD "
- "declared as function", tmpl);
+ error_at (id_loc, "specialization of variable template "
+ "%qD declared as function", tmpl);
inform (DECL_SOURCE_LOCATION (tmpl),
"variable template declared here");
return error_mark_node;
Index: cp/decl2.c
===
--- cp/decl2.c (revision 276646)
+++ cp/decl2.c (working copy)
@@ -1608,7 +1608,8 @@ build_anon_union_vars (tree type, tree object)
just give an error. */
if (TREE_CODE (type) != UNION_TYPE)
{
- error ("anonymous struct not inside named type");
+ error_at (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (type)),
+ "anonymous struct not inside named type");
return error_mark_node;
}
Index: cp/parser.c
===
--- cp/parser.c (revision 276646)
+++ cp/parser.c (working copy)
@@ -9014,6 +9014,7 @@ cp_parser_delete_expression (cp_parser* parser)
bool global_scope_p;
bool array_p;
tree expression;
+ location_t start_loc = cp_lexer_peek_token (parser->lexer)->location;
/* Look for the optional `::' operator. */
global_scope_p
@@ -9043,8 +9044,18 @@ cp_parser_delete_expression (cp_parser* parser)
if (cp_parser_non_integral_constant_expression (parser, NIC_DEL))
return error_mark_node;
- return delete_sanity (expression, NULL_TREE, array_p, global_scope_p,
- tf_warning_or_error);
+ /* Construct a location e.g.:
+ delete [ ] ptr
+ ^~
+ with caret == start at the start of the "delete" token, and
+ the end at the end of the final token we consumed. */
+ location_t combined_loc = make_location (start_loc, start_loc,
+ parser->lexer);
+ expression = delete_sanity (expression, NULL_TREE, array_p,
+ global_scope_p, tf_warning_or_error);
+ protected_set_expr_location (expression, combined_loc);
+
+ return expression;
}
/* Returns 1 if TOKEN may start a cast-expression and isn't '++', '--',
@@ -25827,6 +25838,7 @@ cp_parser_throw_expression (cp_parser* parser)
{
tree expression;
cp_token* token;
+ location_t start_loc = cp_lexer_peek_token (parser->lexer)->location;
cp_parser_require_keyword (parser, RID_THROW, RT_THROW);
token = cp_lexer_peek_token (parser->lexer);
@@ -25842,7 +25854,17 @@ cp_parser_throw_expression (cp_parser* parser)
else
expression = cp_parser_assignment_expression (parser);
- return build_throw (expression);
+ /* Construct a location e.g.:
+ throw x
+ ^~~
+ with caret == start at the start of the "throw" toke
Re: [C++ Patch] Improve grokdeclarator error
Hi,
On 30/09/19 20:46, Jason Merrill wrote:
On Mon, Sep 30, 2019 at 6:54 AM Paolo Carlini wrote:
On 28/09/19 04:05, Jason Merrill wrote:
On 9/26/19 3:39 PM, Paolo Carlini wrote:
+template void foo( // { dg-error "15:template-id .foo.
used as a declarator" }
That's a strange diagnostic message; there's nothing wrong with using
a template-id as a declarator. Why are we even calling grokdeclarator
when we hit EOF in the middle of the declarator?
It's indeed a weird situation. Note, by the way, that for
template void foo;
we end up giving the same diagnostic, only, the location was already fine.
Anyway, to explain why I say weird, clang believes it's dealing with an
explicit instantiation:
explicit instantiation of 'foo' does not refer to a function template,
variable template, member function, member class, or static data member
whereas EDG gives:
declaration is incompatible with function template "void foo<>()"
I *think* what we are issuing is closer in spirit to the latter, we
don't get fooled into thinking it's an instantiation and we say that as
a declaration doesn't work either. See what I mean?
Well, it is an explicit instantiation, the problem (as EDG says) is
that the instantiation declaration doesn't match the template.
Definitely it doesn't.
Removing completely
the diagnostic code doesn't seem fine either because we end up with very
confusing wordings like
variable or field declared void
or worse we mention variable templates, depending on the type (I already
mentioned this).
Any thought about my question about why we're calling grokdeclarator
in the first place?
Yes. If you look at cp_parser_explicit_instantiation, clearly
cp_parser_declarator doesn't return a cp_error_declarator and in such
cases we always call grokdeclarator. Do you think we should error out
here instead, much earlier? Currently grokdeclarator does quite a bit of
work before the diagnostic.
Paolo..
[C++ Patch] More cp_expr_loc_or_input_loc and DECL_SOURCE_LOCATION uses
Hi,
a few more in init.c and name-lookup.c, respectively. Tested
x86_64-linux, as usual.
Thanks, Paolo.
///
/cp
2019-09-30 Paolo Carlini
* init.c (build_new): Use cp_expr_loc_or_input_loc in two places.
* name-lookup.c (do_pushdecl): Use DECL_SOURCE_LOCATION.
(push_class_level_binding_1): Likewise.
(set_decl_namespace): Likewise.
/testsuite
2019-09-30 Paolo Carlini
* g++.dg/diagnostic/integral-array-size-1.C: New.
* g++.dg/cpp0x/alias-decl-1.C: Test location(s) too.
* g++.dg/init/new43.C: Likewise.
* g++.dg/lookup/friend12.C: Likewise.
* g++.dg/lookup/pr79766.C: Likewise.
* g++.dg/lookup/pr84375.C: Likewise.
* g++.dg/other/new-size-type.C: Likewise.
Index: cp/init.c
===
--- cp/init.c (revision 276151)
+++ cp/init.c (working copy)
@@ -3759,7 +3759,8 @@ build_new (vec **placement, tree type
if (!build_expr_type_conversion (WANT_INT | WANT_ENUM, nelts, false))
{
if (complain & tf_error)
-permerror (input_location, "size in array new must have integral
type");
+ permerror (cp_expr_loc_or_input_loc (nelts),
+ "size in array new must have integral type");
else
return error_mark_node;
}
@@ -3774,7 +3775,8 @@ build_new (vec **placement, tree type
less than zero. ... If the expression is a constant expression,
the program is ill-fomed. */
if (TREE_CODE (cst_nelts) == INTEGER_CST
- && !valid_array_size_p (input_location, cst_nelts, NULL_TREE,
+ && !valid_array_size_p (cp_expr_loc_or_input_loc (nelts),
+ cst_nelts, NULL_TREE,
complain & tf_error))
return error_mark_node;
Index: cp/name-lookup.c
===
--- cp/name-lookup.c(revision 276151)
+++ cp/name-lookup.c(working copy)
@@ -3080,8 +3080,9 @@ do_pushdecl (tree decl, bool is_friend)
/* In a local class, a friend function declaration must
find a matching decl in the innermost non-class scope.
[class.friend/11] */
- error ("friend declaration %qD in local class without "
-"prior local declaration", decl);
+ error_at (DECL_SOURCE_LOCATION (decl),
+ "friend declaration %qD in local class without "
+ "prior local declaration", decl);
/* Don't attempt to push it. */
return error_mark_node;
}
@@ -4451,9 +4452,9 @@ push_class_level_binding_1 (tree name, tree x)
tree scope = context_for_name_lookup (x);
if (TYPE_P (scope) && same_type_p (scope, current_class_type))
{
- error ("%qD has the same name as the class in which it is "
-"declared",
-x);
+ error_at (DECL_SOURCE_LOCATION (x),
+ "%qD has the same name as the class in which it is "
+ "declared", x);
return false;
}
}
@@ -4757,7 +4758,8 @@ set_decl_namespace (tree decl, tree scope, bool fr
/* Writing "N::i" to declare something directly in "N" is invalid. */
if (CP_DECL_CONTEXT (decl) == current_namespace
&& at_namespace_scope_p ())
- error ("explicit qualification in declaration of %qD", decl);
+ error_at (DECL_SOURCE_LOCATION (decl),
+ "explicit qualification in declaration of %qD", decl);
return;
}
Index: testsuite/g++.dg/cpp0x/alias-decl-1.C
===
--- testsuite/g++.dg/cpp0x/alias-decl-1.C (revision 276151)
+++ testsuite/g++.dg/cpp0x/alias-decl-1.C (working copy)
@@ -12,5 +12,5 @@ template struct Ptr {}; // { dg-error
struct A {
using A = int; // { dg-error "11:ISO C\\+\\+ forbids nested type .A." }
-// { dg-error "same name as" "" { target c++11 } .-1 }
+// { dg-error "11:.using A = int. has the same name as" "" { target c++11 }
.-1 }
};
Index: testsuite/g++.dg/diagnostic/integral-array-size-1.C
===
--- testsuite/g++.dg/diagnostic/integral-array-size-1.C (nonexistent)
+++ testsuite/g++.dg/diagnostic/integral-array-size-1.C (working copy)
@@ -0,0 +1,7 @@
+template
+void foo(T a)
+{
+ new int[a]; // { dg-error "11:size in array new must have integral type" }
+}
+
+template void foo(float);
Index: testsuite/g++.dg/init/new43.C
Re: [C++ Patch] Improve grokdeclarator error
Hi,
On 28/09/19 04:05, Jason Merrill wrote:
On 9/26/19 3:39 PM, Paolo Carlini wrote:
+template void foo( // { dg-error "15:template-id .foo.
used as a declarator" }
That's a strange diagnostic message; there's nothing wrong with using
a template-id as a declarator. Why are we even calling grokdeclarator
when we hit EOF in the middle of the declarator?
It's indeed a weird situation. Note, by the way, that for
template void foo;
we end up giving the same diagnostic, only, the location was already fine.
Anyway, to explain why I say weird, clang believes it's dealing with an
explicit instantiation:
explicit instantiation of 'foo' does not refer to a function template,
variable template, member function, member class, or static data member
whereas EDG gives:
declaration is incompatible with function template "void foo<>()"
I *think* what we are issuing is closer in spirit to the latter, we
don't get fooled into thinking it's an instantiation and we say that as
a declaration doesn't work either. See what I mean? Removing completely
the diagnostic code doesn't seem fine either because we end up with very
confusing wordings like
variable or field declared void
or worse we mention variable templates, depending on the type (I already
mentioned this).
Thus, all in all, would it make sense to simply issue something closer
to EDG?
Thanks, Paolo.
[C++ Patch] Improve grokdeclarator error
Hi,
over the last weeks, while working on various batches of location
improvements (a new one is forthcoming, in case you are wondering ;) I
noticed this grokdeclarator diagnostic, one of those not exercised by
our testsuite. While constructing a testcase I realized that probably
it's better to immediately return error_mark_node, and avoid an
additional redundant error about variable or field declared void or even
about "fancy" variable templates, depending on the return type of the
function template. Tested x86_64-linux.
Thanks, Paolo.
//
/cp
2019-09-26 Paolo Carlini
* decl.c (grokdeclarator): Immediately return error_mark_node
upon error about template-id used as a declarator.
/testsuite
2019-09-26 Paolo Carlini
* g++.dg/diagnostic/template-id-as-declarator-1.C: New.
Index: testsuite/g++.dg/diagnostic/template-id-as-declarator-1.C
===
--- testsuite/g++.dg/diagnostic/template-id-as-declarator-1.C (nonexistent)
+++ testsuite/g++.dg/diagnostic/template-id-as-declarator-1.C (working copy)
@@ -0,0 +1,5 @@
+template
+void foo() {}
+
+template void foo( // { dg-error "15:template-id .foo. used as a
declarator" }
+// { dg-error "expected" "" { target *-*-* } .-1 }
Index: cp/decl.c
===
--- cp/decl.c (revision 276151)
+++ cp/decl.c (working copy)
@@ -12078,9 +12078,9 @@ grokdeclarator (const cp_declarator *declarator,
&& !FUNC_OR_METHOD_TYPE_P (type)
&& !variable_template_p (TREE_OPERAND (unqualified_id, 0)))
{
- error ("template-id %qD used as a declarator",
-unqualified_id);
- unqualified_id = dname;
+ error_at (id_loc, "template-id %qD used as a declarator",
+ unqualified_id);
+ return error_mark_node;
}
/* If TYPE is a FUNCTION_TYPE, but the function name was explicitly
[C++ Patch] Use DECL_SOURCE_LOCATION more in name-lookup.c
Hi,
Marek's recent fix prompted an audit of name-lookup.c and I found a few
additional straightforward places where we should use a more accurate
location. Tested x86_64-linux.
Thanks, Paolo.
///
/cp
2019-09-24 Paolo Carlini
* name-lookup.c (check_extern_c_conflict): Use DECL_SOURCE_LOCATION.
(check_local_shadow): Use it in three additional places.
/testsuite
2019-09-24 Paolo Carlini
* g++.dg/diagnostic/redeclaration-1.C: New.
* g++.dg/lookup/extern-c-hidden.C: Test location(s) too.
* g++.dg/lookup/extern-c-redecl.C: Likewise.
* g++.dg/lookup/extern-c-redecl6.C: Likewise.
* g++.old-deja/g++.other/using9.C: Likewise.
Index: cp/name-lookup.c
===
--- cp/name-lookup.c(revision 276104)
+++ cp/name-lookup.c(working copy)
@@ -2549,12 +2549,12 @@ check_extern_c_conflict (tree decl)
if (mismatch)
{
auto_diagnostic_group d;
- pedwarn (input_location, 0,
+ pedwarn (DECL_SOURCE_LOCATION (decl), 0,
"conflicting C language linkage declaration %q#D", decl);
inform (DECL_SOURCE_LOCATION (old),
"previous declaration %q#D", old);
if (mismatch < 0)
- inform (input_location,
+ inform (DECL_SOURCE_LOCATION (decl),
"due to different exception specifications");
}
else
@@ -2674,7 +2674,8 @@ check_local_shadow (tree decl)
/* ARM $8.3 */
if (b->kind == sk_function_parms)
{
- error ("declaration of %q#D shadows a parameter", decl);
+ error_at (DECL_SOURCE_LOCATION (decl),
+ "declaration of %q#D shadows a parameter", decl);
return;
}
}
@@ -2700,7 +2701,8 @@ check_local_shadow (tree decl)
&& (old_scope->kind == sk_cond || old_scope->kind == sk_for))
{
auto_diagnostic_group d;
- error ("redeclaration of %q#D", decl);
+ error_at (DECL_SOURCE_LOCATION (decl),
+ "redeclaration of %q#D", decl);
inform (DECL_SOURCE_LOCATION (old),
"%q#D previously declared here", old);
return;
@@ -2723,7 +2725,8 @@ check_local_shadow (tree decl)
&& in_function_try_handler))
{
auto_diagnostic_group d;
- if (permerror (input_location, "redeclaration of %q#D", decl))
+ if (permerror (DECL_SOURCE_LOCATION (decl),
+"redeclaration of %q#D", decl))
inform (DECL_SOURCE_LOCATION (old),
"%q#D previously declared here", old);
return;
Index: testsuite/g++.dg/diagnostic/redeclaration-1.C
===
--- testsuite/g++.dg/diagnostic/redeclaration-1.C (nonexistent)
+++ testsuite/g++.dg/diagnostic/redeclaration-1.C (working copy)
@@ -0,0 +1,20 @@
+void
+foo (int i)
+{
+ int i // { dg-error "7:declaration of .int i. shadows a parameter" }
+(0);
+
+ for (int j ;;)
+int j // { dg-error "9:redeclaration of .int j." }
+ (0);
+}
+
+void
+bar (int i)
+ try
+{ }
+ catch (...)
+{
+ int i // { dg-error "11:redeclaration of .int i." }
+ (0);
+}
Index: testsuite/g++.dg/lookup/extern-c-hidden.C
===
--- testsuite/g++.dg/lookup/extern-c-hidden.C (revision 276104)
+++ testsuite/g++.dg/lookup/extern-c-hidden.C (working copy)
@@ -4,8 +4,8 @@ extern "C" float fabsf (float); // { dg-message "
namespace Bob
{
- extern "C" float fabsf (float, float); // { dg-error "C language" }
+ extern "C" float fabsf (float, float); // { dg-error "20:conflicting C
language" }
extern "C" double fabs (double, double); // { dg-message "previous
declaration" }
}
-extern "C" double fabs (double); // { dg-error "C language" }
+extern "C" double fabs (double); // { dg-error "19:conflicting C language" }
Index: testsuite/g++.dg/lookup/extern-c-redecl.C
===
--- testsuite/g++.dg/lookup/extern-c-redecl.C (revision 276104)
+++ testsuite/g++.dg/lookup/extern-c-redecl.C (working copy)
@@ -8,4 +8,4 @@ namespace A {
// next line should trigger an error because
// it conflicts with previous declaration of foo_func (), due to
// different exception specifications.
-extern "C" void foo_func (); // { dg-error "C language linkage|exception
specifications" }
+extern "C" void foo_func (); // { dg-error "17:confl
[C++ Patch] Use cp_expr_loc_or_input_loc in a few places in pt.c
Hi,
simply a few new uses, in the places where we are dealing with
expressions: we are able to do the right thing in a pretty large number
of additional cases - the below could even include more testcases, most
however are rather tangled (eg, we finally get the locations completely
right only for the c++17 errors, there are additional errors on the same
line, a few redundant ones, etc).
Tested x86_64-linux, as usual.
Thanks, Paolo.
/cp
2019-09-23 Paolo Carlini
* pt.c (check_explicit_specialization): Use cp_expr_loc_or_input_loc.
(process_partial_specialization): Likewise.
(convert_nontype_argument_function): Likewise.
(invalid_tparm_referent_p): Likewise.
(convert_template_argument): Likewise.
(check_valid_ptrmem_cst_expr): Tidy.
/testsuite
2019-09-23 Paolo Carlini
* g++.dg/cpp0x/pr68724.C: Check location(s) too.
* g++.dg/cpp0x/variadic38.C: Likewise.
* g++.dg/cpp1z/nontype2.C: Likewise.
* g++.dg/parse/explicit1.C: Likewise.
* g++.dg/template/crash11.C: Likewise.
* g++.dg/template/non-dependent8.C: Likewise.
* g++.dg/template/nontype-array1.C: Likewise.
* g++.dg/template/nontype3.C: Likewise.
* g++.dg/template/nontype8.C: Likewise.
* g++.dg/template/partial5.C: Likewise.
* g++.dg/template/spec33.C: Likewise.
* g++.old-deja/g++.pt/memtemp64.C: Likewise.
* g++.old-deja/g++.pt/spec20.C: Likewise.
* g++.old-deja/g++.pt/spec21.C: Likewise.
* g++.old-deja/g++.robertl/eb103.C: Likewise.
Index: cp/pt.c
===
--- cp/pt.c (revision 276015)
+++ cp/pt.c (working copy)
@@ -2808,8 +2808,9 @@ check_explicit_specialization (tree declarator,
/* This case handles bogus declarations like template <>
template void f(); */
- error ("template-id %qD in declaration of primary template",
-declarator);
+ error_at (cp_expr_loc_or_input_loc (declarator),
+ "template-id %qE in declaration of primary template",
+ declarator);
return decl;
}
}
@@ -2867,8 +2868,9 @@ check_explicit_specialization (tree declarator,
template void f(); */
if (!uses_template_parms (TREE_OPERAND (declarator, 1)))
- error ("template-id %qD in declaration of primary template",
- declarator);
+ error_at (cp_expr_loc_or_input_loc (declarator),
+ "template-id %qE in declaration of primary template",
+ declarator);
else if (variable_template_p (TREE_OPERAND (declarator, 0)))
{
/* Partial specialization of variable template. */
@@ -2877,11 +2879,13 @@ check_explicit_specialization (tree declarator,
goto ok;
}
else if (cxx_dialect < cxx14)
- error ("non-type partial specialization %qD "
- "is not allowed", declarator);
+ error_at (cp_expr_loc_or_input_loc (declarator),
+ "non-type partial specialization %qE "
+ "is not allowed", declarator);
else
- error ("non-class, non-variable partial specialization %qD "
- "is not allowed", declarator);
+ error_at (cp_expr_loc_or_input_loc (declarator),
+ "non-class, non-variable partial specialization %qE "
+ "is not allowed", declarator);
return decl;
ok:;
}
@@ -4958,8 +4962,9 @@ process_partial_specialization (tree decl)
{
if ((!packed_args && tpd.arg_uses_template_parms[i])
|| (packed_args && uses_template_parms (arg)))
-error ("template argument %qE involves template parameter(s)",
- arg);
+ error_at (cp_expr_loc_or_input_loc (arg),
+ "template argument %qE involves template "
+ "parameter(s)", arg);
else
{
/* Look at the corresponding template parameter,
@@ -6258,13 +6263,14 @@ convert_nontype_argument_function (tree type, tree
{
if (complain & tf_error)
{
- error ("%qE is not a valid template argument for type %qT",
-expr, type);
+ location_t loc = cp_expr_loc_or_input_loc (expr);
+ error_at (loc, "%qE is not a valid template argument for type %qT",
+ expr, type);
if (TYPE_PTR_P (type))
- inform (input_location, "it must be the ad
Re: [C++ Patch] Another bunch of location fixes
Hi, On 15/09/19 16:22, Jason Merrill wrote: On 9/12/19 9:41 AM, Paolo Carlini wrote: + if (!valid_array_size_p (dname + ? declarator->id_loc : input_location, Use the id_loc local variable? This diagnostic is inside the loop over declarator->declarator. Eventually, outside the loop, the id_loc local is updated to the final declarator->id_loc or input_location. Norrmally in the loop we use the current declarator->id_loc: what I tested seems more correct to me (we have to account for input_location too because valid_array_size_p, shared with the C front-end, wants a sound location) Paolo.
[C++ Patch] Another bunch of location fixes
Hi,
nothing special here, various bits I missed so far or in the past meant
to test more thoroughly.
Thanks, Paolo.
//
/cp
2019-09-12 Paolo Carlini
* decl.c (grokdeclarator): Use declspecs->locations and
declarator->id_loc in a few error messages.
* pt.c (finish_member_template_decl): Use DECL_SOURCE_LOCATION.
(push_template_decl_real): Likewise.
/testsuite
2019-09-12 Paolo Carlini
* g++.dg/ext/int128-6.C: New.
* c-c++-common/pr68107.c: Test location(s).
* g++.dg/other/large-size-array.C: Likewise.
* g++.dg/template/dtor2.C: Likewise.
* g++.dg/template/error9.C: Likewise.
* g++.dg/tls/diag-2.C: Likewise.
* g++.dg/tls/diag-4.C: Likewise.
* g++.dg/tls/diag-5.C: Likewise.
* g++.old-deja/g++.pt/memtemp71.C: Likewise.
Index: cp/decl.c
===
--- cp/decl.c (revision 275681)
+++ cp/decl.c (working copy)
@@ -10948,14 +10948,15 @@ grokdeclarator (const cp_declarator *declarator,
{
if (! int_n_enabled_p[declspecs->int_n_idx])
{
- error ("%<__int%d%> is not supported by this target",
-int_n_data[declspecs->int_n_idx].bitsize);
+ error_at (declspecs->locations[ds_type_spec],
+ "%<__int%d%> is not supported by this target",
+ int_n_data[declspecs->int_n_idx].bitsize);
explicit_intN = false;
}
/* Don't pedwarn if the alternate "__intN__" form has been used instead
of "__intN". */
else if (!int_n_alt && pedantic && ! in_system_header_at
(input_location))
- pedwarn (input_location, OPT_Wpedantic,
+ pedwarn (declspecs->locations[ds_type_spec], OPT_Wpedantic,
"ISO C++ does not support %<__int%d%> for %qs",
int_n_data[declspecs->int_n_idx].bitsize, name);
}
@@ -11330,7 +11331,10 @@ grokdeclarator (const cp_declarator *declarator,
&& storage_class != sc_static)
|| typedef_p))
{
- error ("multiple storage classes in declaration of %qs", name);
+ location_t loc
+ = min_location (declspecs->locations[ds_thread],
+ declspecs->locations[ds_storage_class]);
+ error_at (loc, "multiple storage classes in declaration of %qs", name);
thread_p = false;
}
if (decl_context != NORMAL
@@ -11489,7 +11493,9 @@ grokdeclarator (const cp_declarator *declarator,
type = create_array_type_for_decl (dname, type,
declarator->u.array.bounds,
declarator->id_loc);
- if (!valid_array_size_p (input_location, type, dname))
+ if (!valid_array_size_p (dname
+ ? declarator->id_loc : input_location,
+ type, dname))
type = error_mark_node;
if (declarator->std_attributes)
Index: cp/pt.c
===
--- cp/pt.c (revision 275681)
+++ cp/pt.c (working copy)
@@ -298,7 +298,8 @@ finish_member_template_decl (tree decl)
return NULL_TREE;
}
else if (TREE_CODE (decl) == FIELD_DECL)
-error ("data member %qD cannot be a member template", decl);
+error_at (DECL_SOURCE_LOCATION (decl),
+ "data member %qD cannot be a member template", decl);
else if (DECL_TEMPLATE_INFO (decl))
{
if (!DECL_TEMPLATE_SPECIALIZATION (decl))
@@ -310,7 +311,8 @@ finish_member_template_decl (tree decl)
return decl;
}
else
-error ("invalid member template declaration %qD", decl);
+error_at (DECL_SOURCE_LOCATION (decl),
+ "invalid member template declaration %qD", decl);
return error_mark_node;
}
@@ -5515,7 +5517,8 @@ push_template_decl_real (tree decl, bool is_friend
/* [temp.mem]
A destructor shall not be a member template. */
- error ("destructor %qD declared as member template", decl);
+ error_at (DECL_SOURCE_LOCATION (decl),
+ "destructor %qD declared as member template", decl);
return error_mark_node;
}
if (IDENTIFIER_NEWDEL_OP_P (DECL_NAME (decl))
Index: testsuite/c-c++-common/pr68107.c
===
--- testsuite/c-c++-common/pr68107.c(revision 275681)
+++ testsuite/c-c++-common/pr68107.c(working copy)
@@ -3,34 +3,34 @@
#define N ((__SIZE_MAX__ / sizeof (int)) / 2 + 1)
-typedef int (*T1)[N]; /* { dg-error "exceeds maximum object size" } */
+typedef int (*T1)[N]; /* { d
Re: C++ PATCH for c++/91740 - ICE with constexpr call and ?: in ARRAY_REF
Hi again, On 12/09/19 11:03, Paolo Carlini wrote: Hi, On 11/09/19 23:15, Marek Polacek wrote: --- gcc/cp/pt.c +++ gcc/cp/pt.c @@ -26709,7 +26709,7 @@ build_non_dependent_expr (tree expr) if (TREE_CODE (expr) == COND_EXPR) return build3 (COND_EXPR, TREE_TYPE (expr), - TREE_OPERAND (expr, 0), + build_non_dependent_expr (TREE_OPERAND (expr, 0)), (TREE_OPERAND (expr, 1) ? build_non_dependent_expr (TREE_OPERAND (expr, 1)) : build_non_dependent_expr (TREE_OPERAND (expr, 0))), Looks like we would end up unnecessarily calling build_non_dependent_expr three times instead of two: probably is very cheap, probably the code is cleaner this way but I'm a little annoyed at this anyway, for the record ;) Sorry, I misread the code: normally TREE_OPERAND (expr, 1) isn't NULL_TREE thus we are fine. Paolo.
Re: C++ PATCH for c++/91740 - ICE with constexpr call and ?: in ARRAY_REF
Hi, On 11/09/19 23:15, Marek Polacek wrote: --- gcc/cp/pt.c +++ gcc/cp/pt.c @@ -26709,7 +26709,7 @@ build_non_dependent_expr (tree expr) if (TREE_CODE (expr) == COND_EXPR) return build3 (COND_EXPR, TREE_TYPE (expr), - TREE_OPERAND (expr, 0), + build_non_dependent_expr (TREE_OPERAND (expr, 0)), (TREE_OPERAND (expr, 1) ? build_non_dependent_expr (TREE_OPERAND (expr, 1)) : build_non_dependent_expr (TREE_OPERAND (expr, 0))), Looks like we would end up unnecessarily calling build_non_dependent_expr three times instead of two: probably is very cheap, probably the code is cleaner this way but I'm a little annoyed at this anyway, for the record ;) Cheers, Paolo.
[C++ Patch ping] Bunch of location improvements
Hi, On 02/09/19 16:28, Paolo Carlini wrote: Hi, all should be more or less straightforward. I also propose to use an additional range for that error message about constinit && constexpr mentioned to Marek a few days ago. Tested x86_64-linux. I'm gently piniging this very early because the first time I forgot to add the C++ Patch tag. https://gcc.gnu.org/ml/gcc-patches/2019-09/msg00063.html Cheers, Paolo.
Bunch of location improvements
Hi,
all should be more or less straightforward. I also propose to use an
additional range for that error message about constinit && constexpr
mentioned to Marek a few days ago. Tested x86_64-linux.
Thanks, Paolo.
/
/cp
2019-09-02 Paolo Carlini
* decl.c (has_designator_problem): Use cp_expr_loc_or_input_loc
in error_at.
(build_enumerator): Likewise.
(cp_finish_decl): Use DECL_SOURCE_LOCATION.
(grokdeclarator): Use id_loc in two error_at; change errror
message about constinit together constexpr to use two ranges.
/testsuite
2019-09-02 Paolo Carlini
* g++.dg/cpp0x/enum29.C: Test location(s) too.
* g++.dg/cpp0x/lambda/lambda-ice10.C: Likewise.
* g++.dg/cpp2a/constinit3.C: Likewise.
* g++.dg/ext/desig4.C: Likewise.
* g++.dg/ext/label10.C: Likewise.
* g++.old-deja/g++.other/dtor3.C: Likewise.
Index: cp/decl.c
===
--- cp/decl.c (revision 275318)
+++ cp/decl.c (working copy)
@@ -6108,8 +6108,9 @@ has_designator_problem (reshape_iter *d, tsubst_fl
if (d->cur->index)
{
if (complain & tf_error)
- error ("C99 designator %qE outside aggregate initializer",
- d->cur->index);
+ error_at (cp_expr_loc_or_input_loc (d->cur->index),
+ "C99 designator %qE outside aggregate initializer",
+ d->cur->index);
else
return true;
}
@@ -7282,8 +7283,9 @@ cp_finish_decl (tree decl, tree init, bool init_co
if ((flags & LOOKUP_CONSTINIT)
&& !(dk == dk_thread || dk == dk_static))
{
- error ("% can only be applied to a variable with static "
-"or thread storage duration");
+ error_at (DECL_SOURCE_LOCATION (decl),
+ "% can only be applied to a variable with "
+ "static or thread storage duration");
return;
}
@@ -10622,8 +10624,9 @@ grokdeclarator (const cp_declarator *declarator,
&& !uniquely_derived_from_p (ctype,
current_class_type))
{
- error ("invalid use of qualified-name %<%T::%D%>",
- qualifying_scope, decl);
+ error_at (id_declarator->id_loc,
+ "invalid use of qualified-name %<%T::%D%>",
+ qualifying_scope, decl);
return error_mark_node;
}
}
@@ -10810,8 +10813,9 @@ grokdeclarator (const cp_declarator *declarator,
keywords shall appear in a decl-specifier-seq." */
if (constinit_p && constexpr_p)
{
- error_at (min_location (declspecs->locations[ds_constinit],
- declspecs->locations[ds_constexpr]),
+ gcc_rich_location richloc (declspecs->locations[ds_constinit]);
+ richloc.add_range (declspecs->locations[ds_constexpr]);
+ error_at (,
"can use at most one of the % and % "
"specifiers");
return error_mark_node;
@@ -11815,7 +11819,8 @@ grokdeclarator (const cp_declarator *declarator,
&& inner_declarator->u.id.sfk == sfk_destructor
&& arg_types != void_list_node)
{
- error ("destructors may not have parameters");
+ error_at (declarator->id_loc,
+ "destructors may not have parameters");
arg_types = void_list_node;
parms = NULL_TREE;
}
@@ -15155,8 +15160,9 @@ build_enumerator (tree name, tree value, tree enum
if (! INTEGRAL_OR_UNSCOPED_ENUMERATION_TYPE_P
(TREE_TYPE (value)))
{
- error ("enumerator value for %qD must have integral or "
-"unscoped enumeration type", name);
+ error_at (cp_expr_loc_or_input_loc (value),
+ "enumerator value for %qD must have integral or "
+ "unscoped enumeration type", name);
value = NULL_TREE;
}
else
Index: testsuite/g++.dg/cpp0x/enum29.C
===
--- testsuite/g++.dg/cpp0x/enum29.C (revision 275318)
+++ testsuite/g++.dg/cpp0x/enum29.C (working copy)
@@ -38,7 +38,7 @@ enum E0 { e0 = X0() };
enum E1 { e1 = X1() };
enum E2 { e2 = X2() };
enum E3 { e3 = X3() };
-e
Re: C++ PATCH to implement C++20 P1143R2, constinit (PR c++/91360)
Hi,
On 14/08/19 23:22, Marek Polacek wrote:
+ /* [dcl.spec]/2 "At most one of the constexpr, consteval, and constinit
+ keywords shall appear in a decl-specifier-seq." */
+ if (constinit_p && constexpr_p)
+{
+ error_at (min_location (declspecs->locations[ds_constinit],
+ declspecs->locations[ds_constexpr]),
+ "can use at most one of the % and % "
+ "specifiers");
For this error we also have the option of using a gcc_rich_location, and
add_range, etc, like for signed_p && unsigned_p, for example. Just
saying, since we have the infrastructure ready...
Paolo.
[C++ Patch] Improve check_var_type locations
Hi,
by adding a location_t parameter we can improve the locations of the
error messages. At the moment the locations of the first and third
message end up being input_location anyway, but we can imagine improving
those later (this a much more general issue, the locations we use when
unnamed entities are involved, see for example all the 'if (name)' in
grokdeclarator and elsewhere: input_location is very rarely the best
location).
Testex x86_64-linux.
Thanks, Paolo.
/
/cp
2019-08-27 Paolo Carlini
* decl.c (check_var_type): Add location_t parameter and use it.
(grokdeclarator): Adjust call.
* pt.c (tsubst_decl): Likewise.
* cp-tree.h: Adjust declaration.
/testsuite
2019-08-27 Paolo Carlini
* g++.dg/spellcheck-typenames.C: Adjust expected locations.
* g++.dg/cpp0x/pr84676.C: Check locations.
* g++.dg/other/pr88187.C: Likewise.
* g++.dg/parse/crash13.C: Likewise.
* g++.dg/parse/crash46.C: Likewise.
* g++.dg/parse/template28.C: Likewise.
* g++.dg/parse/typename4.C: Likewise.
Index: cp/cp-tree.h
===
--- cp/cp-tree.h(revision 274945)
+++ cp/cp-tree.h(working copy)
@@ -6469,7 +6469,7 @@ extern tree cxx_comdat_group (tree);
extern bool cp_missing_noreturn_ok_p (tree);
extern bool is_direct_enum_init(tree, tree);
extern void initialize_artificial_var (tree, vec *);
-extern tree check_var_type (tree, tree);
+extern tree check_var_type (tree, tree, location_t);
extern tree reshape_init(tree, tree, tsubst_flags_t);
extern tree next_initializable_field (tree);
extern tree fndecl_declared_return_type(tree);
Index: cp/decl.c
===
--- cp/decl.c (revision 274945)
+++ cp/decl.c (working copy)
@@ -10278,19 +10278,20 @@ check_special_function_return_type (special_functi
error-recovery purposes. */
tree
-check_var_type (tree identifier, tree type)
+check_var_type (tree identifier, tree type, location_t loc)
{
if (VOID_TYPE_P (type))
{
if (!identifier)
- error ("unnamed variable or field declared void");
+ error_at (loc, "unnamed variable or field declared void");
else if (identifier_p (identifier))
{
gcc_assert (!IDENTIFIER_ANY_OP_P (identifier));
- error ("variable or field %qE declared void", identifier);
+ error_at (loc, "variable or field %qE declared void",
+ identifier);
}
else
- error ("variable or field declared void");
+ error_at (loc, "variable or field declared void");
type = error_mark_node;
}
@@ -12407,7 +12408,7 @@ grokdeclarator (const cp_declarator *declarator,
error message later. */
if (decl_context != PARM)
{
- type = check_var_type (unqualified_id, type);
+ type = check_var_type (unqualified_id, type, id_loc);
if (type == error_mark_node)
return error_mark_node;
}
Index: cp/pt.c
===
--- cp/pt.c (revision 274945)
+++ cp/pt.c (working copy)
@@ -13894,7 +13895,8 @@ tsubst_decl (tree t, tree args, tsubst_flags_t com
/* Wait until cp_finish_decl to set this again, to handle
circular dependency (template/instantiate6.C). */
DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (r) = 0;
- type = check_var_type (DECL_NAME (r), type);
+ type = check_var_type (DECL_NAME (r), type,
+ DECL_SOURCE_LOCATION (r));
if (DECL_HAS_VALUE_EXPR_P (t))
{
Index: testsuite/g++.dg/cpp0x/pr84676.C
===
--- testsuite/g++.dg/cpp0x/pr84676.C(revision 274945)
+++ testsuite/g++.dg/cpp0x/pr84676.C(working copy)
@@ -1,4 +1,5 @@
// { dg-do compile { target c++11 } }
int a;
-void b(__attribute__((c([](int *) {} (a == (0 = auto)); // { dg-error "" }
+void b(__attribute__((c([](int *) {} (a == (0 = auto)); // { dg-error
"6:variable or field .b. declared void" }
+// { dg-error "expected" "" { target c++11 } .-1 }
Index: testsuite/g++.dg/other/pr88187.C
===
--- testsuite/g++.dg/other/pr88187.C(revision 274945)
+++ testsuite/g++.dg/other/pr88187.C(working copy)
@@ -2,6 +2,6 @@
// { dg-do compile }
template struct A;
-void f (A ()); // { dg-error "variable or field 'f' declared void" "" { target
c++14_down } }
+void f (A ()); // { dg-error "6:variable or field 'f' declared v
[C++ Patch] Fix finish_switch_cond location
Hi,
a rather straightforward tweak. We could imagine much more heavily
reworking the code to avoid saving the original condition
(build_expr_type_conversion return NULL_TREE upon error), I'm not sure
if it's worth it. Tested x86_64-linux.
Thanks, Paolo.
/cp
2019-08-23 Paolo Carlini
* semantics.c (finish_switch_cond): Improve error message location.
/testsuite
2019-08-23 Paolo Carlini
* g++.dg/conversion/simd4.C: Test all the locations.
Index: cp/semantics.c
===
--- cp/semantics.c (revision 274845)
+++ cp/semantics.c (working copy)
@@ -1185,10 +1185,12 @@ finish_switch_cond (tree cond, tree switch_stmt)
if (!processing_template_decl)
{
/* Convert the condition to an integer or enumeration type. */
+ tree orig_cond = cond;
cond = build_expr_type_conversion (WANT_INT | WANT_ENUM, cond, true);
if (cond == NULL_TREE)
{
- error ("switch quantity not an integer");
+ error_at (cp_expr_loc_or_input_loc (orig_cond),
+ "switch quantity not an integer");
cond = error_mark_node;
}
/* We want unlowered type here to handle enum bit-fields. */
Index: testsuite/g++.dg/conversion/simd4.C
===
--- testsuite/g++.dg/conversion/simd4.C (revision 274845)
+++ testsuite/g++.dg/conversion/simd4.C (working copy)
@@ -20,15 +20,15 @@ foo ()
v[b];// { dg-error "4:invalid types" }
w[b];// { dg-error "4:invalid types" }
new int[t];
- new int[u]; // { dg-error "new-declarator must have integral" }
- new int[v]; // { dg-error "new-declarator must have integral" }
- new int[w]; // { dg-error "new-declarator must have integral" }
+ new int[u]; // { dg-error "11:expression in new-declarator must have
integral" }
+ new int[v]; // { dg-error "11:expression in new-declarator must have
integral" }
+ new int[w]; // { dg-error "11:expression in new-declarator must have
integral" }
switch (t) { default: break; }
- switch (u) { default: break; } // { dg-error "switch quantity not an
integer" }
- switch (v) { default: break; } // { dg-error "switch quantity not an
integer" }
- switch (w) { default: break; } // { dg-error "switch quantity not an
integer" }
+ switch (u) { default: break; } // { dg-error "11:switch quantity not
an integer" }
+ switch (v) { default: break; } // { dg-error "11:switch quantity not
an integer" }
+ switch (w) { default: break; } // { dg-error "11:switch quantity not
an integer" }
t = ~t;
- u = ~u; // { dg-error "wrong type argument to bit-complement" }
+ u = ~u; // { dg-error "8:wrong type argument to bit-complement" }
v = ~v;
- w = ~w; // { dg-error "wrong type argument to bit-complement" }
+ w = ~w; // { dg-error "8:wrong type argument to bit-complement" }
}
[C++ Patch] Improve grok_array_decl location
Hi,
for this error message about the types involved in array subscripting I
think we can exploit the available location argument and point to the
open square bracket instead of simply using input_location. Tested
x86_64-linux.
Thanks, Paolo Carlini.
/cp
2019-08-13 Paolo Carlini
* decl2.c (grok_array_decl): Use the location of the open square
bracket in error message about invalid types.
/testsuite
2019-08-13 Paolo Carlini
* g++.dg/conversion/simd4.C: Test locations.
Index: cp/decl2.c
===
--- cp/decl2.c (revision 274330)
+++ cp/decl2.c (working copy)
@@ -434,8 +434,8 @@ grok_array_decl (location_t loc, tree array_expr,
array_expr = p2, index_exp = i1;
else
{
- error ("invalid types %<%T[%T]%> for array subscript",
-type, TREE_TYPE (index_exp));
+ error_at (loc, "invalid types %<%T[%T]%> for array subscript",
+ type, TREE_TYPE (index_exp));
return error_mark_node;
}
Index: testsuite/g++.dg/conversion/simd4.C
===
--- testsuite/g++.dg/conversion/simd4.C (revision 274330)
+++ testsuite/g++.dg/conversion/simd4.C (working copy)
@@ -12,13 +12,13 @@ void
foo ()
{
b[t];
- b[u];// { dg-error "invalid types" }
- b[v];// { dg-error "invalid types" }
- b[w];// { dg-error "invalid types" }
+ b[u];// { dg-error "4:invalid types" }
+ b[v];// { dg-error "4:invalid types" }
+ b[w];// { dg-error "4:invalid types" }
t[b];
- u[b];// { dg-error "invalid types" }
- v[b];// { dg-error "invalid types" }
- w[b];// { dg-error "invalid types" }
+ u[b];// { dg-error "4:invalid types" }
+ v[b];// { dg-error "4:invalid types" }
+ w[b];// { dg-error "4:invalid types" }
new int[t];
new int[u]; // { dg-error "new-declarator must have integral" }
new int[v]; // { dg-error "new-declarator must have integral" }
Re: [C++ Patch] Improve start_function and grokmethod locations
Hi,
On 08/08/19 16:51, Jason Merrill wrote:
On 8/6/19 8:28 AM, Paolo Carlini wrote:
apparently this is now easy to do, likely because a while ago I made
sure that we consistently have meaningful locations for TYPE_DECLs too.
(I went through grokdeclarator and confirmed that when the third
argument is FUNCDEF or MEMFUNCDEF it cannot return NULL_TREE)
-typedef void foo () {} // { dg-error "invalid function
declaration" }
+typedef void foo () {} // { dg-error "14:invalid function
declaration" }
struct S
{
- typedef int bar (void) { return 0; } // { dg-error "invalid member
function declaration" }
+ typedef int bar (void) { return 0; } // { dg-error "15:invalid
member function declaration" }
Maybe we could give a more specific diagnostic in grokdeclarator;
perhaps under
if (typedef_p && decl_context != TYPENAME)
complain and return error_mark_node in FUNCDEF or MEMFUNCDEF context.
Yes, I briefly considered that myself, I only stopped because
grokdeclarator seemed big enough already ;)
Anyway, I tested on x86_64-linux the below. Not 100% sure about the
wording, but we have something similar a few lines below. Also, probably
a single error_at both for functions and member functions would be good
enough (but it would be a specificity regression).
Thanks, Paolo.
Index: cp/decl.c
===
--- cp/decl.c (revision 274220)
+++ cp/decl.c (working copy)
@@ -12165,6 +12165,17 @@ grokdeclarator (const cp_declarator *declarator,
bool alias_p = decl_spec_seq_has_spec_p (declspecs, ds_alias);
tree decl;
+ if (funcdef_flag)
+ {
+ if (decl_context == NORMAL)
+ error_at (id_loc,
+ "typedef may not be a function definition");
+ else
+ error_at (id_loc,
+ "typedef may not be a member function definition");
+ return error_mark_node;
+ }
+
/* This declaration:
typedef void f(int) const;
@@ -15775,13 +15786,6 @@ start_function (cp_decl_specifier_seq *declspecs,
invoke_plugin_callbacks (PLUGIN_START_PARSE_FUNCTION, decl1);
if (decl1 == error_mark_node)
return false;
- /* If the declarator is not suitable for a function definition,
- cause a syntax error. */
- if (decl1 == NULL_TREE || TREE_CODE (decl1) != FUNCTION_DECL)
-{
- error ("invalid function declaration");
- return false;
-}
if (DECL_MAIN_P (decl1))
/* main must return int. grokfndecl should have corrected it
@@ -16436,12 +16440,6 @@ grokmethod (cp_decl_specifier_seq *declspecs,
if (fndecl == error_mark_node)
return error_mark_node;
- if (fndecl == NULL || TREE_CODE (fndecl) != FUNCTION_DECL)
-{
- error ("invalid member function declaration");
- return error_mark_node;
-}
-
if (attrlist)
cplus_decl_attributes (, attrlist, 0);
Index: testsuite/g++.dg/parse/typedef9.C
===
--- testsuite/g++.dg/parse/typedef9.C (revision 274218)
+++ testsuite/g++.dg/parse/typedef9.C (working copy)
@@ -1,8 +1,8 @@
// PR c++/38794
// { dg-do compile }
-typedef void foo () {} // { dg-error "invalid function declaration" }
+typedef void foo () {} // { dg-error "14:typedef may not be a function
definition" }
struct S
{
- typedef int bar (void) { return 0; } // { dg-error "invalid member function
declaration" }
+ typedef int bar (void) { return 0; } // { dg-error "15:typedef may not be a
member function definition" }
};
[C++ Patch] More grokdeclarator locations fixes
Hi,
these are all more or less straightforward, exploit the existing
infrastructure. Nits: the first change also removes an 'a' in the
wording because all the other error messages - a few - mentioning class
or namespace don't have it; the fixes using EXPR_LOCATION also change
%qD to %qE because we are dealing with BIT_NOT_EXPRs.
Thanks, Paolo.
///
/cp
2019-08-08 Paolo Carlini
* decl.c (grokdeclarator): Use id_loc and EXPR_LOCATION in
a few error messages.
/testsuite
2019-08-08 Paolo Carlini
* g++.dg/cpp0x/enum20.C: Test location(s) too.
* g++.dg/other/friend3.C: Likewise.
* g++.dg/parse/dtor5.C: Likewise.
* g++.dg/parse/friend7.C: Likewise.
* g++.dg/template/error22.C: Likewise.
* g++.old-deja/g++.brendan/err-msg5.C: Likewise.
Index: cp/decl.c
===
--- cp/decl.c (revision 274195)
+++ cp/decl.c (working copy)
@@ -10579,7 +10579,8 @@ grokdeclarator (const cp_declarator *declarator,
ctype = qualifying_scope;
if (!MAYBE_CLASS_TYPE_P (ctype))
{
- error ("%q#T is not a class or a namespace", ctype);
+ error_at (id_declarator->id_loc,
+ "%q#T is not a class or namespace", ctype);
ctype = NULL_TREE;
}
else if (innermost_code != cdk_function
@@ -10601,13 +10602,15 @@ grokdeclarator (const cp_declarator *declarator,
{
if (innermost_code != cdk_function)
{
- error ("declaration of %qD as non-function", decl);
+ error_at (EXPR_LOCATION (decl),
+ "declaration of %qE as non-function", decl);
return error_mark_node;
}
else if (!qualifying_scope
&& !(current_class_type && at_class_scope_p ()))
{
- error ("declaration of %qD as non-member", decl);
+ error_at (EXPR_LOCATION (decl),
+ "declaration of %qE as non-member", decl);
return error_mark_node;
}
@@ -12510,7 +12513,7 @@ grokdeclarator (const cp_declarator *declarator,
else if (in_namespace && !friendp)
{
/* Something like struct S { int N::j; }; */
- error ("invalid use of %<::%>");
+ error_at (id_loc, "invalid use of %<::%>");
return error_mark_node;
}
else if (FUNC_OR_METHOD_TYPE_P (type) && unqualified_id)
@@ -12565,15 +12568,15 @@ grokdeclarator (const cp_declarator *declarator,
if (!ctype)
{
gcc_assert (friendp);
- error ("expected qualified name in friend declaration "
- "for destructor %qD", uqname);
+ error_at (id_loc, "expected qualified name in friend "
+ "declaration for destructor %qD", uqname);
return error_mark_node;
}
if (!check_dtor_name (ctype, TREE_OPERAND (uqname, 0)))
{
- error ("declaration of %qD as member of %qT",
- uqname, ctype);
+ error_at (id_loc, "declaration of %qD as member of %qT",
+ uqname, ctype);
return error_mark_node;
}
if (concept_p)
Index: testsuite/g++.dg/cpp0x/enum20.C
===
--- testsuite/g++.dg/cpp0x/enum20.C (revision 274195)
+++ testsuite/g++.dg/cpp0x/enum20.C (working copy)
@@ -2,4 +2,4 @@
// { dg-do compile { target c++11 } }
enum A { };
-void A::f() { }// { dg-error "not a class" }
+void A::f() { }// { dg-error "6:.enum A. is not a
class" }
Index: testsuite/g++.dg/other/friend3.C
===
--- testsuite/g++.dg/other/friend3.C(revision 274195)
+++ testsuite/g++.dg/other/friend3.C(working copy)
@@ -4,10 +4,10 @@
struct A
{
- friend ~A(); // { dg-error "qualified name" }
+ friend ~A(); // { dg-error "10:expected qualified name" }
};
struct B
{
- friend ~A(); // { dg-error "qualified name" }
+ friend ~A(); // { dg-error "10:expected qualified name" }
};
Index: testsuite/g++.dg/parse/dtor5.C
==
Re: C++ PATCH for c++/91264 - detect modifying const objects in constexpr
Hi,
On 31/07/19 21:26, Marek Polacek wrote:
+static void
+modifying_const_object_error (tree expr, tree obj)
+{
+ location_t loc = cp_expr_loc_or_loc (expr, input_location);
Nit: now we have cp_expr_loc_input_loc
Paolo.
[C++ Patch] Improve start_function and grokmethod locations
Hi,
apparently this is now easy to do, likely because a while ago I made
sure that we consistently have meaningful locations for TYPE_DECLs too.
(I went through grokdeclarator and confirmed that when the third
argument is FUNCDEF or MEMFUNCDEF it cannot return NULL_TREE)
Tested x86_64-linux.
Thanks, Paolo.
/cp
2019-08-06 Paolo Carlini
* decl.c (start_function): Use DECL_SOURCE_LOCATION.
(grokmethod): Likewise.
/testsuite
2019-08-06 Paolo Carlini
* g++.dg/parse/typedef9.C: Test locations too.
Index: cp/decl.c
===
--- cp/decl.c (revision 274141)
+++ cp/decl.c (working copy)
@@ -15774,9 +15774,10 @@ start_function (cp_decl_specifier_seq *declspecs,
return false;
/* If the declarator is not suitable for a function definition,
cause a syntax error. */
- if (decl1 == NULL_TREE || TREE_CODE (decl1) != FUNCTION_DECL)
+ if (TREE_CODE (decl1) != FUNCTION_DECL)
{
- error ("invalid function declaration");
+ error_at (DECL_SOURCE_LOCATION (decl1),
+ "invalid function declaration");
return false;
}
@@ -16433,9 +16434,10 @@ grokmethod (cp_decl_specifier_seq *declspecs,
if (fndecl == error_mark_node)
return error_mark_node;
- if (fndecl == NULL || TREE_CODE (fndecl) != FUNCTION_DECL)
+ if (TREE_CODE (fndecl) != FUNCTION_DECL)
{
- error ("invalid member function declaration");
+ error_at (DECL_SOURCE_LOCATION (fndecl),
+ "invalid member function declaration");
return error_mark_node;
}
Index: testsuite/g++.dg/parse/typedef9.C
===
--- testsuite/g++.dg/parse/typedef9.C (revision 274140)
+++ testsuite/g++.dg/parse/typedef9.C (working copy)
@@ -1,8 +1,8 @@
// PR c++/38794
// { dg-do compile }
-typedef void foo () {} // { dg-error "invalid function declaration" }
+typedef void foo () {} // { dg-error "14:invalid function declaration" }
struct S
{
- typedef int bar (void) { return 0; } // { dg-error "invalid member function
declaration" }
+ typedef int bar (void) { return 0; } // { dg-error "15:invalid member
function declaration" }
};
[C++ Patch] One more cp_expr_loc_or_input_loc
Hi,
just spotted an additional error which can benefit from
cp_expr_loc_or_input_loc. Tested x86_64-linux.
Thanks, Paolo.
/cp
2019-08-05 Paolo Carlini
* decl.c (check_array_designated_initializer): Use
cp_expr_loc_or_input_loc in one place.
/testsuite
2019-08-05 Paolo Carlini
* g++.dg/cpp0x/desig1.C: Check location too.
Index: cp/decl.c
===
--- cp/decl.c (revision 274124)
+++ cp/decl.c (working copy)
@@ -5520,8 +5520,9 @@ check_array_designated_initializer (constructor_el
sorry ("non-trivial designated initializers not supported");
}
else
- error ("C99 designator %qE is not an integral constant-expression",
- ce->index);
+ error_at (cp_expr_loc_or_input_loc (ce->index),
+ "C99 designator %qE is not an integral constant-expression",
+ ce->index);
return false;
}
Index: testsuite/g++.dg/cpp0x/desig1.C
===
--- testsuite/g++.dg/cpp0x/desig1.C (revision 274123)
+++ testsuite/g++.dg/cpp0x/desig1.C (working copy)
@@ -25,5 +25,5 @@ struct C
constexpr operator SE() const { return SE::se0; }
};
-int c[] = { [C()] = 0 }; // { dg-error "integral constant-expression" }
+int c[] = { [C()] = 0 }; // { dg-error "14:C99 designator .C\\\(\\\). is not
an integral constant-expression" }
// { dg-warning "does not allow C99 designated
initializers" "" { target *-*-* } .-1 }
Re: [C++ Patch] Improve delete_sanity locations
Hi,
On 31/07/19 21:39, David Malcolm wrote:
[snip]
I don't care for "cp_expr_loc_or_loc".
By "_or_here" do you mean "or input_location"?
Calling it "cp_expr_loc_or_input_location" would spell out what it
does, but would be rather long.
Thanks for your feedback. At this point I don't feel very strongly about
the issue, and, well, hopefully it's just temporary material, but I
tested the below which proposes the name cp_expr_loc_or_input_loc. Seems
a good balance to me. What do you think?
Cheers, Paolo.
/
Index: call.c
===
--- call.c (revision 274001)
+++ call.c (working copy)
@@ -4184,7 +4184,7 @@ build_converted_constant_expr_internal (tree type,
conversion *conv;
void *p;
tree t;
- location_t loc = cp_expr_loc_or_loc (expr, input_location);
+ location_t loc = cp_expr_loc_or_input_loc (expr);
if (error_operand_p (expr))
return error_mark_node;
@@ -6961,7 +6961,7 @@ convert_like_real (conversion *convs, tree expr, t
tree totype = convs->type;
diagnostic_t diag_kind;
int flags;
- location_t loc = cp_expr_loc_or_loc (expr, input_location);
+ location_t loc = cp_expr_loc_or_input_loc (expr);
if (convs->bad_p && !(complain & tf_error))
return error_mark_node;
@@ -7481,7 +7481,7 @@ tree
convert_arg_to_ellipsis (tree arg, tsubst_flags_t complain)
{
tree arg_type;
- location_t loc = cp_expr_loc_or_loc (arg, input_location);
+ location_t loc = cp_expr_loc_or_input_loc (arg);
/* [expr.call]
@@ -7789,7 +7789,7 @@ convert_for_arg_passing (tree type, tree val, tsub
"argument of function call might be a candidate "
"for a format attribute");
}
- maybe_warn_parm_abi (type, cp_expr_loc_or_loc (val, input_location));
+ maybe_warn_parm_abi (type, cp_expr_loc_or_input_loc (val));
}
if (complain & tf_warning)
@@ -8595,7 +8595,7 @@ build_over_call (struct z_candidate *cand, int fla
tree type = TREE_TYPE (to);
tree as_base = CLASSTYPE_AS_BASE (type);
tree arg = argarray[1];
- location_t loc = cp_expr_loc_or_loc (arg, input_location);
+ location_t loc = cp_expr_loc_or_input_loc (arg);
if (is_really_empty_class (type, /*ignore_vptr*/true))
{
@@ -9143,7 +9143,7 @@ build_cxx_call (tree fn, int nargs, tree *argarray
tree fndecl;
/* Remember roughly where this call is. */
- location_t loc = cp_expr_loc_or_loc (fn, input_location);
+ location_t loc = cp_expr_loc_or_input_loc (fn);
fn = build_call_a (fn, nargs, argarray);
SET_EXPR_LOCATION (fn, loc);
@@ -11183,7 +11183,7 @@ perform_implicit_conversion_flags (tree type, tree
{
conversion *conv;
void *p;
- location_t loc = cp_expr_loc_or_loc (expr, input_location);
+ location_t loc = cp_expr_loc_or_input_loc (expr);
if (TYPE_REF_P (type))
expr = mark_lvalue_use (expr);
@@ -11532,7 +11532,7 @@ initialize_reference (tree type, tree expr,
{
conversion *conv;
void *p;
- location_t loc = cp_expr_loc_or_loc (expr, input_location);
+ location_t loc = cp_expr_loc_or_input_loc (expr);
if (type == error_mark_node || error_operand_p (expr))
return error_mark_node;
Index: constexpr.c
===
--- constexpr.c (revision 274001)
+++ constexpr.c (working copy)
@@ -1527,7 +1527,7 @@ cxx_eval_internal_function (const constexpr_ctx *c
default:
if (!ctx->quiet)
- error_at (cp_expr_loc_or_loc (t, input_location),
+ error_at (cp_expr_loc_or_input_loc (t),
"call to internal function %qE", t);
*non_constant_p = true;
return t;
@@ -1542,7 +1542,7 @@ cxx_eval_internal_function (const constexpr_ctx *c
if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
{
- location_t loc = cp_expr_loc_or_loc (t, input_location);
+ location_t loc = cp_expr_loc_or_input_loc (t);
tree type = TREE_TYPE (TREE_TYPE (t));
tree result = fold_binary_loc (loc, opcode, type,
fold_convert_loc (loc, type, arg0),
@@ -1584,7 +1584,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx
bool lval,
bool *non_constant_p, bool *overflow_p)
{
- location_t loc = cp_expr_loc_or_loc (t, input_location);
+ location_t loc = cp_expr_loc_or_input_loc (t);
tree fun = get_function_named_in_call (t);
constexpr_call new_call
= { NULL, NULL, NULL, 0, ctx->manifestly_const_eval };
@@ -2580,7 +2580,7 @@ eval_and_check_array_index (const constexpr_ctx *c
tree t, bool allow_one_past,
bool *non_constant_p, bool *overflow_p)
{
- location_t loc = cp_expr_loc_or_loc (t, input_location);
+ location_t loc = cp_expr_loc_or_input_loc (t);
tree ary = TREE_OPERAND (t, 0);
t = TREE_OPERAND (t, 1);
tree index =
[C++ Patch] Do not warn about [[nodiscard]] applied to a constructor
Hi,
in Cologne, during the presentation of P1771R0, Per Sommerlad pointed
out that apparently GCC was already almost doing the right thing - it
accepts [[nodiscard]] on a constructor and then a warning is emitted in
the relevant potentially dangerous situations - but it does first emit a
warning when it encounters the [[nodiscard]] itself. Avoiding the latter
seems easy to me - the below passes testing. Something else?
Thanks, Paolo.
///
/cp
2019-08-01 Paolo Carlini
* tree.c (handle_nodiscard_attribute): Do not warn about nodiscard
applied to a constructor.
/testsuite
2019-08-01 Paolo Carlini
* g++.dg/cpp1z/nodiscard6.C: New.
Index: cp/tree.c
===
--- cp/tree.c (revision 273951)
+++ cp/tree.c (working copy)
@@ -4361,7 +4361,8 @@ handle_nodiscard_attribute (tree *node, tree name,
{
if (TREE_CODE (*node) == FUNCTION_DECL)
{
- if (VOID_TYPE_P (TREE_TYPE (TREE_TYPE (*node
+ if (VOID_TYPE_P (TREE_TYPE (TREE_TYPE (*node)))
+ && !DECL_CONSTRUCTOR_P (*node))
warning_at (DECL_SOURCE_LOCATION (*node),
OPT_Wattributes, "%qE attribute applied to %qD with void "
"return type", name, *node);
Index: testsuite/g++.dg/cpp1z/nodiscard6.C
===
--- testsuite/g++.dg/cpp1z/nodiscard6.C (nonexistent)
+++ testsuite/g++.dg/cpp1z/nodiscard6.C (working copy)
@@ -0,0 +1,11 @@
+// { dg-do compile { target c++11 } }
+
+struct A
+{
+ [[nodiscard]] A();
+};
+
+void foo()
+{
+ A(); // { dg-warning "ignoring return value" }
+}
[C++ Patch] Improve delete_sanity locations
Hi,
this takes care of the four locations, two warnings and two errors. I'm
also adding the missing complain & tf_warning or tf_error guards, I
don't have a SFINAE testcase failing but since the function takes a
tsubst_flags_t argument I think it's the right thing to do. Tested
x86_64-linux.
By the way, shall we add to cp-tree.h, at least temporarily, a:
inline location_t
cp_expr_loc_or_loc (const_tree t)
{
return cp_expr_loc_or_loc (t, input_location);
}
overload? We could use it in a ton of places.
Thanks, Paolo.
//
/cp
2019-07-24 Paolo Carlini
* decl2.c (delete_sanity): Improve diagnostic locations, use
cp_expr_loc_or_loc in four places.
/testsuite
2019-07-24 Paolo Carlini
* g++.dg/diagnostic/delete1.C: New.
Index: cp/decl2.c
===
--- cp/decl2.c (revision 273767)
+++ cp/decl2.c (working copy)
@@ -487,15 +487,19 @@ delete_sanity (tree exp, tree size, bool doing_vec
}
/* An array can't have been allocated by new, so complain. */
- if (TREE_CODE (TREE_TYPE (exp)) == ARRAY_TYPE)
-warning (0, "deleting array %q#E", exp);
+ if (TREE_CODE (TREE_TYPE (exp)) == ARRAY_TYPE
+ && (complain & tf_warning))
+warning_at (cp_expr_loc_or_loc (exp, input_location), 0,
+ "deleting array %q#E", exp);
t = build_expr_type_conversion (WANT_POINTER, exp, true);
if (t == NULL_TREE || t == error_mark_node)
{
- error ("type %q#T argument given to %, expected pointer",
-TREE_TYPE (exp));
+ if (complain & tf_error)
+ error_at (cp_expr_loc_or_loc (exp, input_location),
+ "type %q#T argument given to %, expected pointer",
+ TREE_TYPE (exp));
return error_mark_node;
}
@@ -506,8 +510,10 @@ delete_sanity (tree exp, tree size, bool doing_vec
/* You can't delete functions. */
if (TREE_CODE (TREE_TYPE (type)) == FUNCTION_TYPE)
{
- error ("cannot delete a function. Only pointer-to-objects are "
-"valid arguments to %");
+ if (complain & tf_error)
+ error_at (cp_expr_loc_or_loc (exp, input_location),
+ "cannot delete a function. Only pointer-to-objects are "
+ "valid arguments to %");
return error_mark_node;
}
@@ -514,7 +520,10 @@ delete_sanity (tree exp, tree size, bool doing_vec
/* Deleting ptr to void is undefined behavior [expr.delete/3]. */
if (VOID_TYPE_P (TREE_TYPE (type)))
{
- warning (OPT_Wdelete_incomplete, "deleting %qT is undefined", type);
+ if (complain & tf_warning)
+ warning_at (cp_expr_loc_or_loc (exp, input_location),
+ OPT_Wdelete_incomplete,
+ "deleting %qT is undefined", type);
doing_vec = 0;
}
Index: testsuite/g++.dg/diagnostic/delete1.C
===
--- testsuite/g++.dg/diagnostic/delete1.C (nonexistent)
+++ testsuite/g++.dg/diagnostic/delete1.C (working copy)
@@ -0,0 +1,14 @@
+void f ()
+{
+ int a[1];
+ delete (a); // { dg-warning "11:deleting array" }
+
+ bool b;
+ delete (b); // { dg-error "11:type .bool. argument" }
+
+ void g ();
+ delete (g); // { dg-error "11:cannot delete a function" }
+
+ void* p;
+ delete (p); // { dg-warning "11:deleting .void*." }
+}
Re: [C++ Patch] A few additional location improvements to grokdeclarator and check_tag_decl
Hi,
On 08/07/19 23:44, Jason Merrill wrote:
On 6/23/19 7:58 AM, Paolo Carlini wrote:
+ error_at (smallest_type_location (get_type_quals (declspecs),
+ declspecs->locations),
How about adding a smallest_type_location overload that just takes
declspecs?
Sure. The below has an additional location fixlet which I noticed over
the last days, for "complex invalid for". Tested x86_64-linux, as usual.
Thanks, Paolo.
/cp
2019-07-09 Paolo Carlini
* decl.c (get_type_quals,
smallest_type_location (const cp_decl_specifier_seq*)): New.
(check_tag_decl): Use smallest_type_location in error_at about
multiple types in one declaration.
(grokdeclarator): Use locations[ds_complex] in error_at about
complex invalid; use locations[ds_storage_class] in error_at
about static cdtor; use id_loc in error_at about flexible
array member in union; use get_type_quals.
/testsuite
2019-07-09 Paolo Carlini
* g++.dg/diagnostic/complex-invalid-1.C: New.
* g++.dg/diagnostic/static-cdtor-1.C: Likewise.
* g++.dg/cpp1z/has-unique-obj-representations2.C: Test location
too.
* g++.dg/other/anon-union3.C: Adjust expected location.
* g++.dg/parse/error8.C: Likewise.
Index: cp/decl.c
===
--- cp/decl.c (revision 273227)
+++ cp/decl.c (working copy)
@@ -100,6 +100,7 @@ static tree build_cp_library_fn (tree, enum tree_c
static void store_parm_decls (tree);
static void initialize_local_var (tree, tree);
static void expand_static_init (tree, tree);
+static location_t smallest_type_location (const cp_decl_specifier_seq*);
/* The following symbols are subsumed in the cp_global_trees array, and
listed here individually for documentation purposes.
@@ -4802,6 +4803,24 @@ warn_misplaced_attr_for_class_type (location_t loc
class_type, class_key_or_enum_as_string (class_type));
}
+/* Returns the cv-qualifiers that apply to the type specified
+ by the DECLSPECS. */
+
+static int
+get_type_quals (const cp_decl_specifier_seq *declspecs)
+{
+ int type_quals = TYPE_UNQUALIFIED;
+
+ if (decl_spec_seq_has_spec_p (declspecs, ds_const))
+type_quals |= TYPE_QUAL_CONST;
+ if (decl_spec_seq_has_spec_p (declspecs, ds_volatile))
+type_quals |= TYPE_QUAL_VOLATILE;
+ if (decl_spec_seq_has_spec_p (declspecs, ds_restrict))
+type_quals |= TYPE_QUAL_RESTRICT;
+
+ return type_quals;
+}
+
/* Make sure that a declaration with no declarator is well-formed, i.e.
just declares a tagged type or anonymous union.
@@ -4821,7 +4840,8 @@ check_tag_decl (cp_decl_specifier_seq *declspecs,
bool error_p = false;
if (declspecs->multiple_types_p)
-error ("multiple types in one declaration");
+error_at (smallest_type_location (declspecs),
+ "multiple types in one declaration");
else if (declspecs->redefined_builtin_type)
{
if (!in_system_header_at (input_location))
@@ -10142,6 +10162,13 @@ smallest_type_location (int type_quals, const loca
return min_location (loc, locations[ds_type_spec]);
}
+static location_t
+smallest_type_location (const cp_decl_specifier_seq *declspecs)
+{
+ int type_quals = get_type_quals (declspecs);
+ return smallest_type_location (type_quals, declspecs->locations);
+}
+
/* Check that it's OK to declare a function with the indicated TYPE
and TYPE_QUALS. SFK indicates the kind of special function (if any)
that this function is. OPTYPE is the type given in a conversion
@@ -10407,7 +10434,7 @@ grokdeclarator (const cp_declarator *declarator,
a member function. */
cp_ref_qualifier rqual = REF_QUAL_NONE;
/* cv-qualifiers that apply to the type specified by the DECLSPECS. */
- int type_quals = TYPE_UNQUALIFIED;
+ int type_quals = get_type_quals (declspecs);
tree raises = NULL_TREE;
int template_count = 0;
tree returned_attrs = NULL_TREE;
@@ -10454,13 +10481,6 @@ grokdeclarator (const cp_declarator *declarator,
if (concept_p)
constexpr_p = true;
- if (decl_spec_seq_has_spec_p (declspecs, ds_const))
-type_quals |= TYPE_QUAL_CONST;
- if (decl_spec_seq_has_spec_p (declspecs, ds_volatile))
-type_quals |= TYPE_QUAL_VOLATILE;
- if (decl_spec_seq_has_spec_p (declspecs, ds_restrict))
-type_quals |= TYPE_QUAL_RESTRICT;
-
if (decl_context == FUNCDEF)
funcdef_flag = true, decl_context = NORMAL;
else if (decl_context == MEMFUNCDEF)
@@ -10999,7 +11019,8 @@ grokdeclarator (const cp_declarator *declarator,
if (decl_spec_seq_has_spec_p (declspecs, ds_complex))
{
if (TREE_CODE (type) != INTEGER_TYPE && TREE_CODE (type) != REAL_TYPE)
- error ("complex invalid for %qs", name);
+ error_at (declspecs->locations[ds_complex],
+ "complex invalid for %qs", name);
/* If a modifier
[C++ Patch PING] Re: [C++ Patch] A few additional location improvements to grokdeclarator and check_tag_decl
Hi, On 23/06/19 13:58, Paolo Carlini wrote: Hi, here there are a couple of rather straightforward improvements in the second half of grokdeclarator plus a check_tag_decl change consistent with the other existing case of multiple_types_p diagnostic. Tested x86_64-linux. Gently pinging this... https://gcc.gnu.org/ml/gcc-patches/2019-06/msg01391.html Thanks, Paolo.
Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
Hi,
On 27/06/19 23:19, Jason Merrill wrote:
Ah, thanks. Then perhaps we want to change the CLASSTYPE_FINAL in
build_over_call to resolves_to_fixed_type_p (arg), to also handle the
other reasons we might know the dynamic type of the argument, and
remove the related code from build_new_method_call_1?
You could avoid needing to move the conditional lower by comparing
DECL_CONTEXT (fn) and BINFO_TYPE (cand->conversion_path) rather than
parmtype and TREE_TYPE (converted_arg).
Sorry for late replying, a few days off.
Anyway, great, it looks like we are reaching a nice synthesis. I must
admit that until yesterday I hadn't noticed that Fabien dealt precisely
with using declarations in order to fix c++/11750, thus the existing
check in build_new_method_call_1 is exactly what we need. The below does
that and passes testing, in it I didn't keep the checks of DECL_VINDEX
(fn) && ! (flags & LOOKUP_NONVIRTUAL) which don't seem necessary, might
avoid function calls, though. Let me know...
Thanks, Paolo.
Index: cp/call.c
===
--- cp/call.c (revision 273076)
+++ cp/call.c (working copy)
@@ -8241,10 +8241,17 @@ build_over_call (struct z_candidate *cand, int fla
return error_mark_node;
}
- /* See if the function member or the whole class type is declared
-final and the call can be devirtualized. */
+ /* Optimize away vtable lookup if we know that this
+function can't be overridden. We need to check if
+the context and the type where we found fn are the same,
+actually FN might be defined in a different class
+type because of a using-declaration. In this case, we
+do not want to perform a non-virtual call. Note that
+resolves_to_fixed_type_p checks CLASSTYPE_FINAL too. */
if (DECL_FINAL_P (fn)
- || CLASSTYPE_FINAL (TYPE_METHOD_BASETYPE (TREE_TYPE (fn
+ || (resolves_to_fixed_type_p (arg, 0)
+ && same_type_ignoring_top_level_qualifiers_p
+ (DECL_CONTEXT (fn), BINFO_TYPE (cand->conversion_path
flags |= LOOKUP_NONVIRTUAL;
/* [class.mfct.nonstatic]: If a nonstatic member function of a class
@@ -9845,17 +9852,6 @@ build_new_method_call_1 (tree instance, tree fns,
if (call != error_mark_node)
{
- /* Optimize away vtable lookup if we know that this
-function can't be overridden. We need to check if
-the context and the type where we found fn are the same,
-actually FN might be defined in a different class
-type because of a using-declaration. In this case, we
-do not want to perform a non-virtual call. */
- if (DECL_VINDEX (fn) && ! (flags & LOOKUP_NONVIRTUAL)
- && same_type_ignoring_top_level_qualifiers_p
- (DECL_CONTEXT (fn), BINFO_TYPE (binfo))
- && resolves_to_fixed_type_p (instance, 0))
- flags |= LOOKUP_NONVIRTUAL;
if (explicit_targs)
flags |= LOOKUP_EXPLICIT_TMPL_ARGS;
/* Now we know what function is being called. */
Index: testsuite/g++.dg/other/final4.C
===
--- testsuite/g++.dg/other/final4.C (nonexistent)
+++ testsuite/g++.dg/other/final4.C (working copy)
@@ -0,0 +1,16 @@
+// PR c++/67184
+// { dg-do compile { target c++11 } }
+// { dg-options "-fdump-tree-original" }
+
+struct B
+{
+ virtual void operator()();
+ virtual operator int();
+ virtual int operator++();
+};
+
+struct D final : B { };
+
+void foo(D& d) { d(); int t = d; ++d; }
+
+// { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "original" } }
Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
Hi, On 23/06/19 19:45, Jason Merrill wrote: On 6/23/19 7:53 AM, Paolo Carlini wrote: ... hi again ;) The other day I was having a look at using declarations for this issue and noticed that only a few lines below the de-virtualization check we have to handle functions found by a using declaration, for various reasons. In particular, we know whether we found a function fn where has been declared or in a derived class. Thus the idea: for the purpose of making some progress, in particular all the cases in c++/67184 & co, would it make sense for the time being to simply add a check to the de-virtualization condition restricting it to non-using declarations? See the below (it also moves the conditional a few lines below only for clarity and consistency with the code handling using declarations, no functional impact) What do you think? Hmm, perhaps we should check CLASSTYPE_FINAL in resolves_to_fixed_type_p rather than in build_over_call at all; then the code in build_new_method_call ought to set LOOKUP_NONVIRTUAL when appropriate. I think your suggestion has to do with the initial implementation of this optimization, as contributed by my friend Roberto Agostino: we had the issue that it didn't handle at all user-defined operators and Vincenzo filed c++/53186. Thus, upon your suggestion, we moved the code to build_over_call, the current place: https://gcc.gnu.org/ml/gcc-patches/2012-05/msg00246.html where it catched both member functions and operators. Now - before we get to the details - if I move the CLASSTYPE_FINAL check to resolves_to_fixed_type_p we exactly regress on c++/53186, that is other/final2.C, because resolves_to_fixed_type_p is *never* called. The pending final4.C, also involving operators (I constructed it exactly because I knew operators could be tricky) is also not fixed, but in that case at least resolves_to_fixed_type_p *is* called, only, too late (I think, I more details later, if you like). All the other existing and pending testcases - involving member functions - appear to be Ok, even with a draft implementation of your suggestion (I slapped a 'if (CLASS_TYPE_P (t) && CLASSTYPE_FINAL (t)) return true;' in the middle of resolves_to_fixed_type_p. Thanks, Paolo.
[C++ Patch] A few additional location improvements to grokdeclarator and check_tag_decl
Hi,
here there are a couple of rather straightforward improvements in the
second half of grokdeclarator plus a check_tag_decl change consistent
with the other existing case of multiple_types_p diagnostic. Tested
x86_64-linux.
Thanks, Paolo.
/
/cp
2019-06-23 Paolo Carlini
* decl.c (get_type_quals): New.
(check_tag_decl): Use smallest_type_location and the latter in
error_at about multiple types in one declaration.
(grokdeclarator): Use locations[ds_storage_class] in error_at
about static cdtor; use id_loc in error_at about flexible
array member in union; use get_type_quals.
/testsuite
2019-06-23 Paolo Carlini
* g++.dg/diagnostic/static-cdtor-1.C: New.
* g++.dg/cpp1z/has-unique-obj-representations2.C: Test location
too.
* g++.dg/other/anon-union3.C: Adjust expected location.
* g++.dg/parse/error8.C: Likewise.
Index: cp/decl.c
===
--- cp/decl.c (revision 272584)
+++ cp/decl.c (working copy)
@@ -100,6 +100,7 @@ static tree build_cp_library_fn (tree, enum tree_c
static void store_parm_decls (tree);
static void initialize_local_var (tree, tree);
static void expand_static_init (tree, tree);
+static location_t smallest_type_location (int, const location_t*);
/* The following symbols are subsumed in the cp_global_trees array, and
listed here individually for documentation purposes.
@@ -4802,6 +4803,24 @@ warn_misplaced_attr_for_class_type (location_t loc
class_type, class_key_or_enum_as_string (class_type));
}
+/* Returns the cv-qualifiers that apply to the type specified
+ by the DECLSPECS. */
+
+static int
+get_type_quals (const cp_decl_specifier_seq *declspecs)
+{
+ int type_quals = TYPE_UNQUALIFIED;
+
+ if (decl_spec_seq_has_spec_p (declspecs, ds_const))
+type_quals |= TYPE_QUAL_CONST;
+ if (decl_spec_seq_has_spec_p (declspecs, ds_volatile))
+type_quals |= TYPE_QUAL_VOLATILE;
+ if (decl_spec_seq_has_spec_p (declspecs, ds_restrict))
+type_quals |= TYPE_QUAL_RESTRICT;
+
+ return type_quals;
+}
+
/* Make sure that a declaration with no declarator is well-formed, i.e.
just declares a tagged type or anonymous union.
@@ -4821,7 +4840,9 @@ check_tag_decl (cp_decl_specifier_seq *declspecs,
bool error_p = false;
if (declspecs->multiple_types_p)
-error ("multiple types in one declaration");
+error_at (smallest_type_location (get_type_quals (declspecs),
+ declspecs->locations),
+ "multiple types in one declaration");
else if (declspecs->redefined_builtin_type)
{
if (!in_system_header_at (input_location))
@@ -10403,7 +10424,7 @@ grokdeclarator (const cp_declarator *declarator,
a member function. */
cp_ref_qualifier rqual = REF_QUAL_NONE;
/* cv-qualifiers that apply to the type specified by the DECLSPECS. */
- int type_quals = TYPE_UNQUALIFIED;
+ int type_quals = get_type_quals (declspecs);
tree raises = NULL_TREE;
int template_count = 0;
tree returned_attrs = NULL_TREE;
@@ -10449,13 +10470,6 @@ grokdeclarator (const cp_declarator *declarator,
if (concept_p)
constexpr_p = true;
- if (decl_spec_seq_has_spec_p (declspecs, ds_const))
-type_quals |= TYPE_QUAL_CONST;
- if (decl_spec_seq_has_spec_p (declspecs, ds_volatile))
-type_quals |= TYPE_QUAL_VOLATILE;
- if (decl_spec_seq_has_spec_p (declspecs, ds_restrict))
-type_quals |= TYPE_QUAL_RESTRICT;
-
if (decl_context == FUNCDEF)
funcdef_flag = true, decl_context = NORMAL;
else if (decl_context == MEMFUNCDEF)
@@ -11571,9 +11585,12 @@ grokdeclarator (const cp_declarator *declarator,
virtual. A constructor may not be static.
A constructor may not be declared with ref-qualifier. */
if (staticp == 2)
- error ((flags == DTOR_FLAG)
-? G_("destructor cannot be static member function")
-: G_("constructor cannot be static member function"));
+ error_at (declspecs->locations[ds_storage_class],
+ (flags == DTOR_FLAG)
+ ? G_("destructor cannot be static member "
+"function")
+ : G_("constructor cannot be static member "
+"function"));
if (memfn_quals)
{
error ((flags == DTOR_FLAG)
@@ -12431,7 +12448,7 @@ grokdeclarator (const cp_declarator *declarator,
&& (TREE_CODE (ctype) == UNION_TYPE
|| TREE_CODE (ctype) == QUAL_UNION_TYPE))
{
- error ("flexible array member in union");
+ error_at (id_loc, &qu
Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
... hi again ;)
The other day I was having a look at using declarations for this issue
and noticed that only a few lines below the de-virtualization check we
have to handle functions found by a using declaration, for various
reasons. In particular, we know whether we found a function fn where has
been declared or in a derived class. Thus the idea: for the purpose of
making some progress, in particular all the cases in c++/67184 & co,
would it make sense for the time being to simply add a check to the
de-virtualization condition restricting it to non-using declarations?
See the below (it also moves the conditional a few lines below only for
clarity and consistency with the code handling using declarations, no
functional impact) What do you think?
Thanks, Paolo.
///
Index: cp/call.c
===
--- cp/call.c (revision 272583)
+++ cp/call.c (working copy)
@@ -8241,12 +8241,6 @@ build_over_call (struct z_candidate *cand, int fla
return error_mark_node;
}
- /* See if the function member or the whole class type is declared
-final and the call can be devirtualized. */
- if (DECL_FINAL_P (fn)
- || CLASSTYPE_FINAL (TYPE_METHOD_BASETYPE (TREE_TYPE (fn
- flags |= LOOKUP_NONVIRTUAL;
-
/* [class.mfct.nonstatic]: If a nonstatic member function of a class
X is called for an object that is not of type X, or of a type
derived from X, the behavior is undefined.
@@ -8271,6 +8265,17 @@ build_over_call (struct z_candidate *cand, int fla
else
return error_mark_node;
}
+
+ /* See if the function member or the whole class type is declared
+final and the call can be devirtualized. */
+ if (DECL_FINAL_P (fn)
+ || (CLASSTYPE_FINAL (TREE_TYPE (argtype))
+ /* Give up for now if fn was found by a using declaration,
+the complex case, see c++/90909. */
+ && (TREE_TYPE (TREE_TYPE (converted_arg))
+ == TREE_TYPE (parmtype
+ flags |= LOOKUP_NONVIRTUAL;
+
/* If fn was found by a using declaration, the conversion path
will be to the derived class, not the base declaring fn. We
must convert from derived to base. */
Index: testsuite/g++.dg/other/final3.C
===
--- testsuite/g++.dg/other/final3.C (nonexistent)
+++ testsuite/g++.dg/other/final3.C (working copy)
@@ -0,0 +1,28 @@
+// PR c++/67184
+// { dg-do compile { target c++11 } }
+// { dg-options "-fdump-tree-original" }
+
+struct V {
+ virtual void foo();
+};
+
+struct wV final : V {
+};
+
+struct oV final : V {
+ void foo();
+};
+
+void call(wV& x)
+{
+ x.foo();
+ x.V::foo();
+}
+
+void call(oV& x)
+{
+ x.foo();
+ x.V::foo();
+}
+
+// { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "original" } }
Index: testsuite/g++.dg/other/final4.C
===
--- testsuite/g++.dg/other/final4.C (nonexistent)
+++ testsuite/g++.dg/other/final4.C (working copy)
@@ -0,0 +1,16 @@
+// PR c++/67184
+// { dg-do compile { target c++11 } }
+// { dg-options "-fdump-tree-original" }
+
+struct B
+{
+ virtual void operator()();
+ virtual operator int();
+ virtual int operator++();
+};
+
+struct D final : B { };
+
+void foo(D& d) { d(); int t = d; ++d; }
+
+// { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "original" } }
Index: testsuite/g++.dg/other/final5.C
===
--- testsuite/g++.dg/other/final5.C (nonexistent)
+++ testsuite/g++.dg/other/final5.C (working copy)
@@ -0,0 +1,19 @@
+// PR c++/69445
+// { dg-do compile { target c++11 } }
+// { dg-options "-fdump-tree-original" }
+
+struct Base {
+ virtual void foo() const = 0;
+ virtual void bar() const {}
+};
+
+struct C final : Base {
+ void foo() const { }
+};
+
+void func(const C & c) {
+ c.bar();
+ c.foo();
+}
+
+// { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "original" } }
Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
... so, now that I see the issue more clearly, I'm adding to the
testsuite the below too.
Thanks, Paolo.
Index: final7.C
===
--- final7.C(nonexistent)
+++ final7.C(working copy)
@@ -0,0 +1,11 @@
+// PR c++/90909
+// { dg-do run { target c++11 } }
+
+#include
+
+struct S1 { virtual bool f() { return false; } };
+struct S2: S1 { virtual bool f() { return true; } };
+struct S3: S2 { using S1::f; };
+struct S4 final: S3 { void g(); };
+void S4::g() { assert (f() == true); }
+int main() { S4().g(); }
Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
and... By the way, if S1:.f is not pure virtual, just a virtual declaration - all the rest being the same - the code doesn't link: undefined references to vtable and typeinfo for S1. The same happens with current clang and icc. I don't know if this detail helps us in the short term ... the same happens without final too. Paolo.
Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
Hi, On 21/06/19 21:27, Paolo Carlini wrote: Hi, On 21/06/19 20:50, Jason Merrill wrote: On 6/20/19 12:24 AM, Paolo Carlini wrote: Hi, this bug notices that the more aggressive de-virtualization check that we have now in place (fixed c++/67184) doesn't work correctly for the below reproducer, which involves a pure virtual: we de-virtualize and the build fails at link-time. To cure this I believe we simply want an additional DECL_PURE_VIRTUAL_P in the condition. I don't see why this bug would be specific to pure virtual functions, so the fix also should not be. If S1::f is not pure virtual, I'd expect that we wrongly devirtualize to it in the same way. By the way, if S1:.f is not pure virtual, just a virtual declaration - all the rest being the same - the code doesn't link: undefined references to vtable and typeinfo for S1. The same happens with current clang and icc. I don't know if this detail helps us in the short term Paolo.
Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
Hi, On 21/06/19 20:50, Jason Merrill wrote: On 6/20/19 12:24 AM, Paolo Carlini wrote: Hi, this bug notices that the more aggressive de-virtualization check that we have now in place (fixed c++/67184) doesn't work correctly for the below reproducer, which involves a pure virtual: we de-virtualize and the build fails at link-time. To cure this I believe we simply want an additional DECL_PURE_VIRTUAL_P in the condition. I don't see why this bug would be specific to pure virtual functions, so the fix also should not be. If S1::f is not pure virtual, I'd expect that we wrongly devirtualize to it in the same way. Devirtualizing the call in S4 is a good optimization, we're just selecting the wrong function. It seems like the issue here is that the using-declaration hides the final overrider from lookup. So we need to work harder to find the actual final overrider, perhaps by looking into the argtype vtable. I see, thanks for the suggestion. The issue seems rather tricky, then. For the time being I'm going to revert the recent improvements. Probably I'm also going to unassign myself, because I don't want to prevent somebody else more knowledgeable than me in the area from contributing a good solution. Thanks, Paolo.
[C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
Hi,
this bug notices that the more aggressive de-virtualization check that
we have now in place (fixed c++/67184) doesn't work correctly for the
below reproducer, which involves a pure virtual: we de-virtualize and
the build fails at link-time. To cure this I believe we simply want an
additional DECL_PURE_VIRTUAL_P in the condition. I also checked that the
other compilers I have at hand appear to do the same, that is, they
compile the reproducer both as-is and without the final specifier to the
same assembly.
Note, in principle we have the option of not doing the additional
DECL_PURE_VIRTUAL_P check when the final overrider comes from the class
itself, not from a base, that is in the cases that we were already
de-virtualizing pre-67184. That is, for something like:
struct A final
{
virtual void foo () = 0;
};
void fun(A* a, B* b)
{
a->foo();
}
devirtualize anyway (which then doesn't link). We could add back an '||
CLASSTYPE_FINAL (TYPE_METHOD_BASETYPE (TREE_TYPE (fn)))' for that. ICC
appears to behave this way.
Tested x86_64-linux.
Thanks, Paolo.
/cp
2019-06-20 Paolo Carlini
PR c++/90909
* call.c (build_over_call): Do not try to devirtualize when
then function is pure virtual.
/testsuite
2019-06-20 Paolo Carlini
PR c++/90909
* g++.dg/other/final6.C: New.
Index: testsuite/g++.dg/other/final6.C
===
--- testsuite/g++.dg/other/final6.C (nonexistent)
+++ testsuite/g++.dg/other/final6.C (working copy)
@@ -0,0 +1,9 @@
+// PR c++/90909
+// { dg-do link { target c++11 } }
+
+struct S1 { virtual void f() = 0; };
+struct S2: S1 { virtual void f() {} };
+struct S3: S2 { using S1::f; };
+struct S4 final: S3 { void g(); };
+void S4::g() { f(); }
+int main() { S4().g(); }
Index: cp/call.c
===
--- cp/call.c (revision 272410)
+++ cp/call.c (working copy)
@@ -8244,7 +8244,8 @@ build_over_call (struct z_candidate *cand, int fla
/* See if the function member or the whole class type is declared
final and the call can be devirtualized. */
if (DECL_FINAL_P (fn)
- || CLASSTYPE_FINAL (TREE_TYPE (argtype)))
+ || (CLASSTYPE_FINAL (TREE_TYPE (argtype))
+ && !DECL_PURE_VIRTUAL_P (fn)))
flags |= LOOKUP_NONVIRTUAL;
/* [class.mfct.nonstatic]: If a nonstatic member function of a class
[C++ Patch] More grokdeclarator locations
Hi,
should all be rather straightforward but I spent quite a bit of time on
the testsuite changes... Tested x86_64-linux, as usual.
Thanks, Paolo.
PS: at some point we should start talking about all the error, error_at,
etc, which are *never* exercised by our testsuite. I'm coming to the
conclusion that some should really go. Any general procedural suggestions?
///
/cp
2019-06-14 Paolo Carlini
* decl.c (grokdeclarator): Use id_loc, typespec_loc, and
locations[ds_storage_class] in a few additional places.
/testsuite
2019-06-14 Paolo Carlini
* g++.dg/diagnostic/auto-storage-1.C: New.
* g++.dg/diagnostic/no-type-1.C: Likewise.
* g++.dg/diagnostic/no-type-2.C: Likewise.
* g++.dg/diagnostic/top-level-auto-1.C: Likewise.
* g++.dg/cpp0x/auto9.C: Test some locations too.
* g++.dg/cpp1z/register1.C: Likewise.
* g++.dg/cpp1z/register2.C: Likewise.
* g++.dg/cpp1z/register3.C: Likewise.
* g++.dg/other/error34.C: Likewise.
Index: cp/decl.c
===
--- cp/decl.c (revision 272259)
+++ cp/decl.c (working copy)
@@ -10797,13 +10797,14 @@ grokdeclarator (const cp_declarator *declarator,
else if (in_system_header_at (input_location) || flag_ms_extensions)
/* Allow it, sigh. */;
else if (! is_main)
- permerror (input_location, "ISO C++ forbids declaration of %qs with no
type", name);
+ permerror (id_loc, "ISO C++ forbids declaration of %qs with no type",
+ name);
else if (pedantic)
- pedwarn (input_location, OPT_Wpedantic,
+ pedwarn (id_loc, OPT_Wpedantic,
"ISO C++ forbids declaration of %qs with no type", name);
else
- warning (OPT_Wreturn_type,
-"ISO C++ forbids declaration of %qs with no type", name);
+ warning_at (id_loc, OPT_Wreturn_type,
+ "ISO C++ forbids declaration of %qs with no type", name);
if (type_was_error_mark_node && template_parm_flag)
/* FIXME we should be able to propagate the error_mark_node as is
@@ -11232,7 +11233,8 @@ grokdeclarator (const cp_declarator *declarator,
else if (toplevel_bindings_p ())
{
if (storage_class == sc_auto)
- error ("top-level declaration of %qs specifies %", name);
+ error_at (declspecs->locations[ds_storage_class],
+ "top-level declaration of %qs specifies %", name);
}
else if (thread_p
&& storage_class != sc_extern
@@ -12323,9 +12325,10 @@ grokdeclarator (const cp_declarator *declarator,
&& !(cxx_dialect >= cxx17 && template_parm_flag))
{
if (cxx_dialect >= cxx14)
- error ("% parameter not permitted in this context");
+ error_at (typespec_loc,
+ "% parameter not permitted in this context");
else
- error ("parameter declared %");
+ error_at (typespec_loc, "parameter declared %");
type = error_mark_node;
}
@@ -12746,9 +12749,12 @@ grokdeclarator (const cp_declarator *declarator,
// FIXME:gcc_assert (original_name == dname);
if (storage_class == sc_auto)
- error ("storage class % invalid for function %qs", name);
+ error_at (declspecs->locations[ds_storage_class],
+ "storage class % invalid for function %qs", name);
else if (storage_class == sc_register)
- error ("storage class % invalid for function %qs", name);
+ error_at (declspecs->locations[ds_storage_class],
+ "storage class % invalid for function %qs",
+ name);
else if (thread_p)
{
if (declspecs->gnu_thread_keyword_p)
Index: testsuite/g++.dg/cpp0x/auto9.C
===
--- testsuite/g++.dg/cpp0x/auto9.C (revision 272259)
+++ testsuite/g++.dg/cpp0x/auto9.C (working copy)
@@ -78,10 +78,10 @@ enum struct D : auto * { FF = 0 }; // { dg-error
void
bar ()
{
- try { } catch (auto i) { } // { dg-error "parameter" }
- try { } catch (auto) { } // { dg-error "parameter" }
- try { } catch (auto *i) { } // { dg-error "parameter" }
- try { } catch (auto *) { } // { dg-error "parameter" }
+ try { } catch (auto i) { } // { dg-error "18:parameter" }
+ try { } catch (auto) { } // { dg-error "18:parameter" }
+ try { } catch (auto *i) { } // { dg-error "18:parameter" }
+ try { } catch (auto *)
Re: [C++ Patch] Further grokdeclarator locations work
Hi,
On 12/06/19 22:24, Jason Merrill wrote:
On 6/6/19 9:00 AM, Paolo Carlini wrote:
Hi,
only minor functional changes here - more precise locations for two
error messages - but I think it's a step in the right direction: it
moves the declaration of id_loc way up, near typespec_loc and as such
id_loc is immediately used. Then its value is updated upon the loop
over declarator in the middle of the function and used again in the
final part of the function. That also "frees" the simple name loc for
other local uses, allows to simplify those checks changed rather
recently over (name && declarator). and (unqualified_id &&
declarator). Tested x86_64-linux.
Mostly OK, except:
if (declspecs->multiple_types_p)
{
- error ("two or more data types in declaration of %qs", name);
+ error_at (id_loc, "two or more data types in declaration of
%qs", name);
return error_mark_node;
}
declspecs->locations[ds_type_spec]?
if (declspecs->conflicting_specifiers_p)
{
- error ("conflicting specifiers in declaration of %qs", name);
+ error_at (id_loc, "conflicting specifiers in declaration of
%qs", name);
return error_mark_node;
}
ds_storage_class/ds_typedef?
Ok.
Ideally, it would be nice to have available locations for the two
leftmost data types and, for the second error, the locations of
conflicting storage classes (eg, static and register). Other compilers
then point to the second one, where the issue shows up. I think we
already briefly discussed that kind of logic in the past, but in those
cases we had available all the locations we needed and we ended up using
gcc_rich_location and add_range, etc..
Anyway, we can certainly use typespec_loc and the minimum of
ds_storage_class and ds_typedef, respectively, like in the below, which
passes testing.
Thanks! Paolo.
///
Index: cp/decl.c
===
--- cp/decl.c (revision 272213)
+++ cp/decl.c (working copy)
@@ -10456,6 +10456,8 @@ grokdeclarator (const cp_declarator *declarator,
if (typespec_loc == UNKNOWN_LOCATION)
typespec_loc = input_location;
+ location_t id_loc = declarator ? declarator->id_loc : input_location;
+
/* Look inside a declarator for the name being declared
and get it as a string, for an error message. */
for (id_declarator = declarator;
@@ -10620,8 +10622,7 @@ grokdeclarator (const cp_declarator *declarator,
D1 ( parameter-declaration-clause) ... */
if (funcdef_flag && innermost_code != cdk_function)
{
- error_at (declarator->id_loc,
- "function definition does not declare parameters");
+ error_at (id_loc, "function definition does not declare parameters");
return error_mark_node;
}
@@ -10629,8 +10630,7 @@ grokdeclarator (const cp_declarator *declarator,
&& innermost_code != cdk_function
&& ! (ctype && !declspecs->any_specifiers_p))
{
- error_at (declarator->id_loc,
- "declaration of %qD as non-function", dname);
+ error_at (id_loc, "declaration of %qD as non-function", dname);
return error_mark_node;
}
@@ -10639,8 +10639,7 @@ grokdeclarator (const cp_declarator *declarator,
if (UDLIT_OPER_P (dname)
&& innermost_code != cdk_function)
{
- error_at (declarator->id_loc,
- "declaration of %qD as non-function", dname);
+ error_at (id_loc, "declaration of %qD as non-function", dname);
return error_mark_node;
}
@@ -10648,14 +10647,12 @@ grokdeclarator (const cp_declarator *declarator,
{
if (typedef_p)
{
- error_at (declarator->id_loc,
- "declaration of %qD as %", dname);
+ error_at (id_loc, "declaration of %qD as %", dname);
return error_mark_node;
}
else if (decl_context == PARM || decl_context == CATCHPARM)
{
- error_at (declarator->id_loc,
- "declaration of %qD as parameter", dname);
+ error_at (id_loc, "declaration of %qD as parameter", dname);
return error_mark_node;
}
}
@@ -10705,13 +10702,16 @@ grokdeclarator (const cp_declarator *declarator,
issue an error message. */
if (declspecs->multiple_types_p)
{
- error ("two or more data types in declaration of %qs", name);
+ error_at (typespec_loc,
+ "two or more data types in declaration of %qs", name);
return error_mark_node;
}
if (declspecs->conflicting_specifiers_p)
{
- error ("conflicting specifiers
[C++ Patch] More grokdeclarator locations fixes
Hi,
this one requires my last patch, uses id_loc in a few additional places
in the last part of the function.
Most of the changes should be straightforward, I only want to mention
the "typedef name may not be a nested-name-specifier" change, where, not
using input_location means that for a test like
g++.old-deja/g++.jason/crash10.C:
struct A {
enum foo { bar };
};
typedef A::foo A::foo;
the location is under the 'A' of the second 'A::foo'. The EDG front-end
does exactly the same; clang points to the 'f' but then has the wavy
thing reaching back to 'A' (a while ago I already mentioned that we
don't have that option). Tested x86_64-linux.
Thanks, Paolo.
/
/cp
2019-06-10 Paolo Carlini
* decl.c (grokdeclarator): Use id_loc in five additional places
in the last part of the function.
/testsuite
2019-06-10 Paolo Carlini
* g++.dg/diagnostic/variably-modified-type-1.C: New.
* g++.dg/cpp0x/alias-decl-1.C: Test the location too.
* g++.dg/other/pr84792-1.C: Likewise.
* g++.dg/other/pr84792-2.C: Likewise.
* g++.dg/parse/error24.C: Likewise.
* g++.dg/parse/error32.C: Likewise.
* g++.dg/parse/error33.C: Likewise.
* g++.dg/parse/saved1.C: Likewise.
* g++.dg/template/operator6.C: Likewise.
* g++.dg/template/pr61745.C: Likewise.
* g++.dg/template/typedef41.C: Likewise.
* g++.old-deja/g++.jason/crash10.C: Likewise.
Index: cp/decl.c
===
--- cp/decl.c (revision 272105)
+++ cp/decl.c (working copy)
@@ -12000,9 +12000,11 @@ grokdeclarator (const cp_declarator *declarator,
&& variably_modified_type_p (type, NULL_TREE))
{
if (decl_context == FIELD)
- error ("data member may not have variably modified type %qT", type);
+ error_at (id_loc,
+ "data member may not have variably modified type %qT", type);
else
- error ("parameter may not have variably modified type %qT", type);
+ error_at (id_loc,
+ "parameter may not have variably modified type %qT", type);
type = error_mark_node;
}
@@ -12106,7 +12108,7 @@ grokdeclarator (const cp_declarator *declarator,
if (id_declarator && declarator->u.id.qualifying_scope)
{
- error ("typedef name may not be a nested-name-specifier");
+ error_at (id_loc, "typedef name may not be a nested-name-specifier");
type = error_mark_node;
}
@@ -12130,7 +12132,7 @@ grokdeclarator (const cp_declarator *declarator,
}
else if (current_class_type
&& constructor_name_p (unqualified_id, current_class_type))
- permerror (input_location, "ISO C++ forbids nested type %qD with same
name "
+ permerror (id_loc, "ISO C++ forbids nested type %qD with same name "
"as enclosing class",
unqualified_id);
@@ -12288,7 +12290,7 @@ grokdeclarator (const cp_declarator *declarator,
/* Only functions may be declared using an operator-function-id. */
if (dname && IDENTIFIER_ANY_OP_P (dname))
{
- error ("declaration of %qD as non-function", dname);
+ error_at (id_loc, "declaration of %qD as non-function", dname);
return error_mark_node;
}
Index: testsuite/g++.dg/cpp0x/alias-decl-1.C
===
--- testsuite/g++.dg/cpp0x/alias-decl-1.C (revision 272105)
+++ testsuite/g++.dg/cpp0x/alias-decl-1.C (working copy)
@@ -11,5 +11,6 @@ template using Ptr = U*;
template struct Ptr {}; // { dg-error "specialization" }
struct A {
-using A = int;//{ dg-error "nested|has|same name as|class|in
which|declared" }
+using A = int; // { dg-error "11:ISO C\\+\\+ forbids nested type .A." }
+// { dg-error "same name as" "" { target c++11 } .-1 }
};
Index: testsuite/g++.dg/diagnostic/variably-modified-type-1.C
===
--- testsuite/g++.dg/diagnostic/variably-modified-type-1.C (nonexistent)
+++ testsuite/g++.dg/diagnostic/variably-modified-type-1.C (working copy)
@@ -0,0 +1,12 @@
+// { dg-options "" }
+
+void foo ()
+{
+ int n;
+ typedef int X[n];
+ struct Z
+ {
+X x __attribute__((unused)); // { dg-error "7:data member may not have
variably modified type" }
+void bar (X x __attribute__((unused))); // { dg-error "17:parameter may
not have variably modified type" }
+ };
+}
Index: testsuite/g++.dg/other/pr84792-1.C
===
--- testsuite/g++.dg/other/pr84792-1.C (revision 272105)
+++ test
Re: [C++ PATCH] PR c++/90449 - add -Winaccessible-base option.
Hi, On 07/06/19 22:31, Marek Polacek wrote: On Fri, Jun 07, 2019 at 10:20:02PM +0200, Paolo Carlini wrote: Hi, On 07/06/19 22:10, Matthew Beliveau wrote: @@ -6025,6 +6025,10 @@ warn_about_ambiguous_bases (tree t) Just a nit, but it seems weird to me that the function implementing warn_inaccessible_base is named warn_about_ambiguous_bases. Maybe, but we want to keep the warning's name in sync with clang, and renaming the function seems like unnecessary noise. I could go either way. It's definitely a minor issue. But, given that we have a rationale for inaccessible_base - I didn't know that - I vote for renaming the function. A maybe_* name would be appropriate in that case, because the function doesn't always warn. Paolo.
Re: [C++ PATCH] PR c++/90449 - add -Winaccessible-base option.
Hi, On 07/06/19 22:10, Matthew Beliveau wrote: @@ -6025,6 +6025,10 @@ warn_about_ambiguous_bases (tree t) Just a nit, but it seems weird to me that the function implementing warn_inaccessible_base is named warn_about_ambiguous_bases. Paolo.
[C++ Patch] Further grokdeclarator locations work
Hi,
only minor functional changes here - more precise locations for two
error messages - but I think it's a step in the right direction: it
moves the declaration of id_loc way up, near typespec_loc and as such
id_loc is immediately used. Then its value is updated upon the loop over
declarator in the middle of the function and used again in the final
part of the function. That also "frees" the simple name loc for other
local uses, allows to simplify those checks changed rather recently over
(name && declarator). and (unqualified_id && declarator). Tested
x86_64-linux.
Thanks, Paolo.
//////
/cp
2019-06-06 Paolo Carlini
* decl.c (grokdeclarator): Move further up the declaration of
id_loc, use it immediately, update its value after the loop
over declarator, use it again in the final part of function;
use id_loc in error messages about multiple data types and
conflicting specifiers.
/testsuite
2019-06-06 Paolo Carlini
* g++.dg/diagnostic/conflicting-specifiers-1.C: New.
* g++.dg/diagnostic/two-or-more-data-types-1.C: Likewise.
Index: cp/decl.c
===
--- cp/decl.c (revision 271974)
+++ cp/decl.c (working copy)
@@ -10456,6 +10456,8 @@ grokdeclarator (const cp_declarator *declarator,
if (typespec_loc == UNKNOWN_LOCATION)
typespec_loc = input_location;
+ location_t id_loc = declarator ? declarator->id_loc : input_location;
+
/* Look inside a declarator for the name being declared
and get it as a string, for an error message. */
for (id_declarator = declarator;
@@ -10620,8 +10622,7 @@ grokdeclarator (const cp_declarator *declarator,
D1 ( parameter-declaration-clause) ... */
if (funcdef_flag && innermost_code != cdk_function)
{
- error_at (declarator->id_loc,
- "function definition does not declare parameters");
+ error_at (id_loc, "function definition does not declare parameters");
return error_mark_node;
}
@@ -10629,8 +10630,7 @@ grokdeclarator (const cp_declarator *declarator,
&& innermost_code != cdk_function
&& ! (ctype && !declspecs->any_specifiers_p))
{
- error_at (declarator->id_loc,
- "declaration of %qD as non-function", dname);
+ error_at (id_loc, "declaration of %qD as non-function", dname);
return error_mark_node;
}
@@ -10639,8 +10639,7 @@ grokdeclarator (const cp_declarator *declarator,
if (UDLIT_OPER_P (dname)
&& innermost_code != cdk_function)
{
- error_at (declarator->id_loc,
- "declaration of %qD as non-function", dname);
+ error_at (id_loc, "declaration of %qD as non-function", dname);
return error_mark_node;
}
@@ -10648,14 +10647,12 @@ grokdeclarator (const cp_declarator *declarator,
{
if (typedef_p)
{
- error_at (declarator->id_loc,
- "declaration of %qD as %", dname);
+ error_at (id_loc, "declaration of %qD as %", dname);
return error_mark_node;
}
else if (decl_context == PARM || decl_context == CATCHPARM)
{
- error_at (declarator->id_loc,
- "declaration of %qD as parameter", dname);
+ error_at (id_loc, "declaration of %qD as parameter", dname);
return error_mark_node;
}
}
@@ -10705,13 +10702,13 @@ grokdeclarator (const cp_declarator *declarator,
issue an error message. */
if (declspecs->multiple_types_p)
{
- error ("two or more data types in declaration of %qs", name);
+ error_at (id_loc, "two or more data types in declaration of %qs", name);
return error_mark_node;
}
if (declspecs->conflicting_specifiers_p)
{
- error ("conflicting specifiers in declaration of %qs", name);
+ error_at (id_loc, "conflicting specifiers in declaration of %qs", name);
return error_mark_node;
}
@@ -11861,6 +11858,8 @@ grokdeclarator (const cp_declarator *declarator,
}
}
+ id_loc = declarator ? declarator->id_loc : input_location;
+
/* A `constexpr' specifier used in an object declaration declares
the object as `const'. */
if (constexpr_p && innermost_code != cdk_function)
@@ -11884,8 +11883,6 @@ grokdeclarator (const cp_declarator *declarator,
unqualified_id = dname;
}
- location_t loc = declarator ? declarator->id_loc : input_location;
-
/* If TYPE is a FUNCTION_TYPE, but the function name was explicitly
qualified with a class-name, turn it into a METHOD_TYPE, unless
Re: [C++ Patch] Use declarator->id_loc in three additional places
Hi,
On 05/06/19 19:45, Jason Merrill wrote:
On 6/4/19 11:57 AM, Paolo Carlini wrote:
Hi,
On 04/06/19 16:50, Jason Merrill wrote:
On 6/4/19 10:31 AM, Paolo Carlini wrote:
+ permerror (loc, "member functions are implicitly "
+ "friends of their class");
Wouldn't it be better to use the location of "friend" in this
diagnostic?
Yes, however doing that fully correctly seems a bit tricky
Why tricky? Doesn't declspecs->locations[ds_friend] work?
To be honest, here I wasn't considering ds_friend at all. Indeed it
gives us the location of 'friend', but, say, for a testcase like
parse/friend4.C:
struct A
{
friend void A::foo(); // { dg-error "implicitly friends" }
friend A::~A(); // { dg-error "implicitly friends" }
};
I thought we wanted a precise caret under the 'f' of 'foo' and the '~'
of the destructor - both clang and icc do that - I wasn't even
considering pointing at 'friend'. If you think that would be an
improvement wrt the current closed parenthesis - I agreee it would! - I
can do that!
Paolo.
[C++ Patch] Improve check_special_function_return_type locations
Hi,
here certainly we can do better than using input_location. In principle
we could also pass the location of the entity (constructor, destructor,
etc) itself or something else but I think it makes a lot of sense to
simply include locations[ds_type_spec] in the computation, seems
consistent with the existing case of spurious qualifiers (ICC does
something similar AFAICS). Tested x86_64-linux.
Thanks, Paolo.
//
/cp
2019-06-05 Paolo Carlini
* decl.c (smallest_type_location): Add.
(check_special_function_return_type): Use it.
(grokdeclarator): Lkewise.
/testsuite
2019-06-05 Paolo Carlini
* g++.dg/diagnostic/return-type-invalid-1.C: New.
* g++.old-deja/g++.brendan/crash16.C: Adjust.
* g++.old-deja/g++.law/ctors5.C: Likewise.
Index: cp/decl.c
===
--- cp/decl.c (revision 271949)
+++ cp/decl.c (working copy)
@@ -10111,6 +10111,15 @@ smallest_type_quals_location (int type_quals, cons
return loc;
}
+/* Returns the smallest among the latter and locations[ds_type_spec]. */
+
+static location_t
+smallest_type_location (int type_quals, const location_t* locations)
+{
+ location_t loc = smallest_type_quals_location (type_quals, locations);
+ return min_location (loc, locations[ds_type_spec]);
+}
+
/* Check that it's OK to declare a function with the indicated TYPE
and TYPE_QUALS. SFK indicates the kind of special function (if any)
that this function is. OPTYPE is the type given in a conversion
@@ -10129,7 +10138,8 @@ check_special_function_return_type (special_functi
{
case sfk_constructor:
if (type)
- error ("return type specification for constructor invalid");
+ error_at (smallest_type_location (type_quals, locations),
+ "return type specification for constructor invalid");
else if (type_quals != TYPE_UNQUALIFIED)
error_at (smallest_type_quals_location (type_quals, locations),
"qualifiers are not allowed on constructor declaration");
@@ -10142,7 +10152,8 @@ check_special_function_return_type (special_functi
case sfk_destructor:
if (type)
- error ("return type specification for destructor invalid");
+ error_at (smallest_type_location (type_quals, locations),
+ "return type specification for destructor invalid");
else if (type_quals != TYPE_UNQUALIFIED)
error_at (smallest_type_quals_location (type_quals, locations),
"qualifiers are not allowed on destructor declaration");
@@ -10157,7 +10168,8 @@ check_special_function_return_type (special_functi
case sfk_conversion:
if (type)
- error ("return type specified for %", optype);
+ error_at (smallest_type_location (type_quals, locations),
+ "return type specified for %", optype);
else if (type_quals != TYPE_UNQUALIFIED)
error_at (smallest_type_quals_location (type_quals, locations),
"qualifiers are not allowed on declaration of "
@@ -10168,7 +10180,8 @@ check_special_function_return_type (special_functi
case sfk_deduction_guide:
if (type)
- error ("return type specified for deduction guide");
+ error_at (smallest_type_location (type_quals, locations),
+ "return type specified for deduction guide");
else if (type_quals != TYPE_UNQUALIFIED)
error_at (smallest_type_quals_location (type_quals, locations),
"qualifiers are not allowed on declaration of "
@@ -10438,10 +10451,8 @@ grokdeclarator (const cp_declarator *declarator,
if (initialized > 1)
funcdef_flag = true;
- location_t typespec_loc = smallest_type_quals_location (type_quals,
- declspecs->locations);
- typespec_loc = min_location (typespec_loc,
- declspecs->locations[ds_type_spec]);
+ location_t typespec_loc = smallest_type_location (type_quals,
+ declspecs->locations);
if (typespec_loc == UNKNOWN_LOCATION)
typespec_loc = input_location;
Index: testsuite/g++.dg/diagnostic/return-type-invalid-1.C
===
--- testsuite/g++.dg/diagnostic/return-type-invalid-1.C (nonexistent)
+++ testsuite/g++.dg/diagnostic/return-type-invalid-1.C (working copy)
@@ -0,0 +1,27 @@
+struct S1
+{
+ int S1(); // { dg-error "3:return type" }
+ int ~S1(); // { dg-error "3:return type" }
+ int operator int(); // { dg-error "3:return type" }
+};
+
+struct S2
+{
+ const int S2(); // { dg-error "3:return type" }
+ const int ~S2(); // { dg-error "3:return type" }
+ cons
Re: PR C++/63149
Hi,
On 04/06/19 21:26, Nina Dinka Ranns wrote:
Good point, dg-do compile is sufficient to demonstrate the issue.
I agree.
A couple of additional nits, sorry for mentioning only now.
C++63149_2.diff
Index: gcc/cp/pt.c
===
--- gcc/cp/pt.c (revision 271709)
+++ gcc/cp/pt.c (working copy)
@@ -26836,7 +26836,7 @@
static tree
listify_autos (tree type, tree auto_node)
{
- tree init_auto = listify (auto_node);
+ tree init_auto = listify (strip_top_quals(auto_node));
You want a space after strip_top_quals.
tree argvec = make_tree_vec (1);
TREE_VEC_ELT (argvec, 0) = init_auto;
if (processing_template_decl)
Index: gcc/testsuite/g++.dg/cpp0x/initlist-deduce2.C
===
--- gcc/testsuite/g++.dg/cpp0x/initlist-deduce2.C (nonexistent)
+++ gcc/testsuite/g++.dg/cpp0x/initlist-deduce2.C (working copy)
@@ -0,0 +1,12 @@
+// Test for PR63149
+// { dg-do compile { target c++11 } }
+
+#include
+
+const auto r = { 1, 2, 3 };
+using X = decltype(r);
+using X = const std::initializer_list;
+
+int main()
+{
+}
With dg-do compile you don't need a main anymore.
I seem to remember also a couple of minor formatting issues in the
ChangeLog entry: just harmonize the format with everything else you find
in the ChangeLog, in terms of the usual trivial details: upper cases,
line lenghts and line wraps, etc.
Paolo.
Re: PR C++/63149
Hi,
On 04/06/19 18:36, Nina Dinka Ranns wrote:
+// Test for PR63149
+// { dg-do run { target c++11 } }
Are you sure you want a dg-do run?
Paolo.
Re: [C++ Patch] Use declarator->id_loc in three additional places
Hi, On 04/06/19 16:50, Jason Merrill wrote: On 6/4/19 10:31 AM, Paolo Carlini wrote: + permerror (loc, "member functions are implicitly " + "friends of their class"); Wouldn't it be better to use the location of "friend" in this diagnostic? Yes, however doing that fully correctly seems a bit tricky, I thought that pointing to the id_loc it's still better than a rather meaningless place near the end of the line, eg, before the semicolon. Note this is a more general issue, I'll give it some thought... I'm leaving those friends alone for the time being. Thanks, Paolo.
[C++ Patch] Use declarator->id_loc in three additional places
Hi,
tested x86_64-linux, as usual.
Thanks, Paolo.
///
/cp
2019-06-04 Paolo Carlini
* decl.c (grokdeclarator): Use declarator->id_loc in three
additional places.
/testsuite
2019-06-04 Paolo Carlini
* g++.dg/concepts/pr60573.C: Test locations too.
* g++.dg/cpp0x/deleted13.C: Likewise.
* g++.dg/other/friend4.C: Likewise.
* g++.dg/other/friend5.C: Likewise.
* g++.dg/other/friend7.C: Likewise.
* g++.dg/parse/error29.C: Likewise.
* g++.dg/parse/friend7.C: Likewise.
* g++.dg/parse/qualified4.C: Likewise.
* g++.dg/template/crash96.C Likewise.
* g++.old-deja/g++.brendan/crash22.C Likewise.
* g++.old-deja/g++.brendan/crash23.C Likewise.
* g++.old-deja/g++.law/visibility10.C Likewise.
* g++.old-deja/g++.other/decl5.C: Likewise.
Index: cp/decl.c
===
--- cp/decl.c (revision 271899)
+++ cp/decl.c (working copy)
@@ -11873,6 +11873,8 @@ grokdeclarator (const cp_declarator *declarator,
unqualified_id = dname;
}
+ location_t loc = declarator ? declarator->id_loc : input_location;
+
/* If TYPE is a FUNCTION_TYPE, but the function name was explicitly
qualified with a class-name, turn it into a METHOD_TYPE, unless
we know that the function is static. We take advantage of this
@@ -11893,13 +11895,12 @@ grokdeclarator (const cp_declarator *declarator,
{
if (friendp)
{
- permerror (input_location, "member functions are implicitly "
-"friends of their class");
+ permerror (loc, "member functions are implicitly "
+"friends of their class");
friendp = 0;
}
else
- permerror (declarator->id_loc,
- "extra qualification %<%T::%> on member %qs",
+ permerror (loc, "extra qualification %<%T::%> on member %qs",
ctype, name);
}
else if (/* If the qualifying type is already complete, then we
@@ -11928,19 +11929,19 @@ grokdeclarator (const cp_declarator *declarator,
if (current_class_type
&& (!friendp || funcdef_flag || initialized))
{
- error (funcdef_flag || initialized
-? G_("cannot define member function %<%T::%s%> "
- "within %qT")
-: G_("cannot declare member function %<%T::%s%> "
- "within %qT"),
-ctype, name, current_class_type);
+ error_at (loc, funcdef_flag || initialized
+ ? G_("cannot define member function %<%T::%s%> "
+"within %qT")
+ : G_("cannot declare member function %<%T::%s%> "
+"within %qT"),
+ ctype, name, current_class_type);
return error_mark_node;
}
}
else if (typedef_p && current_class_type)
{
- error ("cannot declare member %<%T::%s%> within %qT",
-ctype, name, current_class_type);
+ error_at (loc, "cannot declare member %<%T::%s%> within %qT",
+ ctype, name, current_class_type);
return error_mark_node;
}
}
@@ -12053,8 +12054,6 @@ grokdeclarator (const cp_declarator *declarator,
}
}
- location_t loc = declarator ? declarator->id_loc : input_location;
-
/* If this is declaring a typedef name, return a TYPE_DECL. */
if (typedef_p && decl_context != TYPENAME)
{
Index: testsuite/g++.dg/concepts/pr60573.C
===
--- testsuite/g++.dg/concepts/pr60573.C (revision 271899)
+++ testsuite/g++.dg/concepts/pr60573.C (working copy)
@@ -9,7 +9,7 @@ struct A
void foo(auto);
};
- void B::foo(auto) {} // { dg-error "cannot define" }
+ void B::foo(auto) {} // { dg-error "8:cannot define" }
struct X
{
@@ -21,8 +21,8 @@ struct A
};
};
-void Y::Z::foo(auto) {} // { dg-error "cannot define" }
+void Y::Z::foo(auto) {} // { dg-error "10:cannot define" }
};
- void X::Y::Z::foo(auto) {} // { dg-error "cannot define" }
+ void X::Y::Z::foo(auto) {} // { dg-error "8:cannot define" }
};
Index: testsuite/g++.dg/cpp0x/deleted13.C
===
--- testsuite/g++.dg/cpp0x/deleted13.C (revision 271899)
+++ testsuite/g++.dg/cpp0x/deleted13.C (working copy)
@@ -8,5 +8,5 @@
Re: [PATCH][Preprocessor][Version 2]patch to fix PR 90581
Hi,
a few minor comments.
On 03/06/19 16:49, Qing Zhao wrote:
+fmax-include-depth=
+C ObjC C++ ObjC++ Joined RejectNegative UInteger
+fmax-include-depth= Set the maximum number of depth of nested #include.
I think you want something like "Set the maximum depth of nested #include".
+@item -fmax-include-depth=@var{depth}
+@opindex fmax-include-depth
+Set the maximum number of depth of the nested include. The default is 200.
Likewise, plus include or #include?
In any case the wording should be consistent with the actual cpp_error
message below in your attachment, thus, in particular, no "number".
+ /* the maximum depths for nested #include. */
Upper case "The". "for" or "of"? Again, consistency.
Paolo.
[C++ Patch] Fix cp_parser_unqualified_id typo
Hi,
noticed this typo: we are passing the location to build_nt as third
argument instead of passing it to cp_expr as second argument. Tested
x86_64-linux.
By the way, we have the option of using build_min_nt_loc instead of
build_nt in all these places for BIT_NOT_EXPR in
cp_parser_unqualified_id, Certainly it has the advantage that the
location information survives in the tree node when cp_expr is
eventually converted to a plain tree... (below tests fine) What do you
think?
Thanks, Paolo.
2019-05-31 Paolo Carlini
* parser.c (cp_parser_unqualified_id): Fix typo, properly call
build_nt.
Index: parser.c
===
--- parser.c(revision 271785)
+++ parser.c(working copy)
@@ -6063,7 +6063,7 @@ cp_parser_unqualified_id (cp_parser* parser,
"%<~auto%> only available with "
"%<-std=c++14%> or %<-std=gnu++14%>");
cp_lexer_consume_token (parser->lexer);
- return cp_expr (build_nt (BIT_NOT_EXPR, make_auto (), loc));
+ return cp_expr (build_nt (BIT_NOT_EXPR, make_auto ()), loc);
}
/* If there was an explicit qualification (S::~T), first look
Index: parser.c
===
--- parser.c(revision 271761)
+++ parser.c(working copy)
@@ -6052,7 +6052,7 @@ cp_parser_unqualified_id (cp_parser* parser,
&& constructor_name_p (token->u.value, scope
{
cp_lexer_consume_token (parser->lexer);
- return cp_expr (build_nt (BIT_NOT_EXPR, scope), loc);
+ return build_min_nt_loc (loc, BIT_NOT_EXPR, scope);
}
/* ~auto means the destructor of whatever the object is. */
@@ -6063,7 +6063,7 @@ cp_parser_unqualified_id (cp_parser* parser,
"%<~auto%> only available with "
"%<-std=c++14%> or %<-std=gnu++14%>");
cp_lexer_consume_token (parser->lexer);
- return cp_expr (build_nt (BIT_NOT_EXPR, make_auto (), loc));
+ return build_min_nt_loc (loc, BIT_NOT_EXPR, make_auto ());
}
/* If there was an explicit qualification (S::~T), first look
@@ -6153,8 +6153,8 @@ cp_parser_unqualified_id (cp_parser* parser,
time. */
type_decl = cp_parser_identifier (parser);
if (type_decl != error_mark_node)
- type_decl = build_nt (BIT_NOT_EXPR, type_decl);
- return cp_expr (type_decl, loc);
+ type_decl = build_min_nt_loc (loc, BIT_NOT_EXPR, type_decl);
+ return type_decl;
}
}
/* If an error occurred, assume that the name of the
@@ -6162,7 +6162,7 @@ cp_parser_unqualified_id (cp_parser* parser,
class. That allows us to keep parsing after running
into ill-formed destructor names. */
if (type_decl == error_mark_node && scope)
- return build_nt (BIT_NOT_EXPR, scope);
+ return build_min_nt_loc (loc, BIT_NOT_EXPR, scope);
else if (type_decl == error_mark_node)
return error_mark_node;
@@ -6189,7 +6189,7 @@ cp_parser_unqualified_id (cp_parser* parser,
"typedef-name %qD used as destructor declarator",
type_decl);
- return cp_expr (build_nt (BIT_NOT_EXPR, TREE_TYPE (type_decl), loc));
+ return build_min_nt_loc (loc, BIT_NOT_EXPR, TREE_TYPE (type_decl));
}
case CPP_KEYWORD:
Re: [C++ Patch] PR 89875 ("[7/8/9/10 Regression] invalid typeof reference to a member of an incomplete struct accepted at function scope")
Hi,
On 28/05/19 16:47, Jason Merrill wrote:
On 5/10/19 10:29 AM, Paolo Carlini wrote:
Hi,
a while ago Martin noticed that an unintended consequence of an old
tweak of mine - which avoided redundant error messages emitted from
cp_parser_init_declarator - is that, in some cases, we started
accepting ill-formed typeofs. Luckily, decltype isn't affected and
that points to the real issue: by the time that place in
cp_parser_init_declarator is reached, for a decltype version we
already emitted a correct error message. Thus I think the right way
to fix the problem is simply committing to tentative parse when,
toward the end of cp_parser_sizeof_operand we know that we must be
looking at a (possibly ill-formed) expression. Tested x86_64-linux.
The problem with calling cp_parser_commit_to_tentative_parse here is
that the tentative parse you're committing to is for the enclosing
scope, which is trying to decide whether e.g. we're parsing a
declaration or expression. If the operand of typeof is a well-formed
expression, and the larger context is an expression, this will break.
Better, I think, to commit and re-parse only if you actually encounter
an error.
Alternately, cp_parser_decltype_expr deals with this by using a
tentative firewall and CPP_DECLTYPE; cp_parser_sizeof_operand could do
the same, but that seems like a bigger hammer.
Today I spent quite a bit of time on this and eventually decided to
follow the example of decltype as closely as possible. Then I started
tweaking those drafts which laready passed the testsuite and after a
while ended up with the below, rather close to the current code, in
fact. Testing !cp_parser_error_occurred and in case calling
cp_parser_abort_tentative_parse by hand (closer to the decltype example)
also works. What do you think? Thanks, Paolo.
///
Index: cp/parser.c
===
--- cp/parser.c (revision 271692)
+++ cp/parser.c (working copy)
@@ -28942,6 +28942,8 @@ cp_parser_sizeof_operand (cp_parser* parser, enum
{
tree type = NULL_TREE;
+ tentative_firewall firewall (parser);
+
/* We can't be sure yet whether we're looking at a type-id or an
expression. */
cp_parser_parse_tentatively (parser);
@@ -28969,11 +28971,15 @@ cp_parser_sizeof_operand (cp_parser* parser, enum
/* If all went well, then we're done. */
if (cp_parser_parse_definitely (parser))
expr = type;
+ else
+ {
+ /* Commit to the tentative_firewall so we get syntax errors. */
+ cp_parser_commit_to_tentative_parse (parser);
+
+ expr = cp_parser_unary_expression (parser);
+ }
}
-
- /* If the type-id production did not work out, then we must be
- looking at the unary-expression production. */
- if (!expr)
+ else
expr = cp_parser_unary_expression (parser);
/* Go back to evaluating expressions. */
Index: testsuite/g++.dg/cpp0x/decltype-pr66548.C
===
--- testsuite/g++.dg/cpp0x/decltype-pr66548.C (revision 271692)
+++ testsuite/g++.dg/cpp0x/decltype-pr66548.C (working copy)
@@ -11,7 +11,7 @@ struct Meow {};
void f ()
{
- decltype (Meow.purr ()) d; // { dg-error "expected primary-expression"
"pr89875" { xfail c++98_only } }
+ decltype (Meow.purr ()) d; // { dg-error "expected primary-expression" }
(void)
}
Index: testsuite/g++.dg/template/sizeof-template-argument.C
===
--- testsuite/g++.dg/template/sizeof-template-argument.C(revision
271692)
+++ testsuite/g++.dg/template/sizeof-template-argument.C(working copy)
@@ -3,9 +3,9 @@
template struct A {};
-template struct B : A {}; /* { dg-error "template
argument" } */
+template struct B : A {}; /* { dg-error "expected
primary-expression" } */
-template struct C : A {}; /* { dg-error "template
argument" } */
+template struct C : A {}; /* { dg-error "expected
primary-expression" } */
int a;
Re: [C++ Patch] PR 89875 ("[7/8/9/10 Regression] invalid typeof reference to a member of an incomplete struct accepted at function scope")
Hi, gently pinging this... On 10/05/19 16:29, Paolo Carlini wrote: Hi, a while ago Martin noticed that an unintended consequence of an old tweak of mine - which avoided redundant error messages emitted from cp_parser_init_declarator - is that, in some cases, we started accepting ill-formed typeofs. Luckily, decltype isn't affected and that points to the real issue: by the time that place in cp_parser_init_declarator is reached, for a decltype version we already emitted a correct error message. Thus I think the right way to fix the problem is simply committing to tentative parse when, toward the end of cp_parser_sizeof_operand we know that we must be looking at a (possibly ill-formed) expression. Tested x86_64-linux. https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00501.html Thanks, Paolo. PS: To be honest, not knowing the exact rules, I would not consider this issue a P2 - in particular considering that typeof is legacy and is affected but all sorts of weird issues - but that's what it is ;)
[C++ Patch] A few more grokdeclarator locations fixes
Hi,
one more, rather straightforward, simply use the location stored in the
declarator. Tested x86_64-linux.
Thanks, Paolo.
/cp
2019-05-23 Paolo Carlini
* decl.c (grokdeclarator): Use declarator->id_loc in five error
error_at calls.
/testsuite
2019-05-23 Paolo Carlini
* g++.dg/cpp0x/alias-decl-18.C: Test location too.
* g++.dg/cpp0x/udlit-nofunc-neg.C: Likewise.
* g++.dg/parse/crash59.C: Likewise.
* g++.dg/parse/error38.C: Likewise.
* g++.dg/parse/error39.C: Likewise.
* g++.dg/template/crash31.C: Likewise.
* g++.dg/template/operator8.C: Likewise.
* g++.dg/template/operator9.C: Likewise.
Index: cp/decl.c
===
--- cp/decl.c (revision 271542)
+++ cp/decl.c (working copy)
@@ -10592,7 +10592,8 @@ grokdeclarator (const cp_declarator *declarator,
D1 ( parameter-declaration-clause) ... */
if (funcdef_flag && innermost_code != cdk_function)
{
- error ("function definition does not declare parameters");
+ error_at (declarator->id_loc,
+ "function definition does not declare parameters");
return error_mark_node;
}
@@ -10600,7 +10601,8 @@ grokdeclarator (const cp_declarator *declarator,
&& innermost_code != cdk_function
&& ! (ctype && !declspecs->any_specifiers_p))
{
- error ("declaration of %qD as non-function", dname);
+ error_at (declarator->id_loc,
+ "declaration of %qD as non-function", dname);
return error_mark_node;
}
@@ -10609,7 +10611,8 @@ grokdeclarator (const cp_declarator *declarator,
if (UDLIT_OPER_P (dname)
&& innermost_code != cdk_function)
{
- error ("declaration of %qD as non-function", dname);
+ error_at (declarator->id_loc,
+ "declaration of %qD as non-function", dname);
return error_mark_node;
}
@@ -10617,12 +10620,14 @@ grokdeclarator (const cp_declarator *declarator,
{
if (typedef_p)
{
- error ("declaration of %qD as %", dname);
+ error_at (declarator->id_loc,
+ "declaration of %qD as %", dname);
return error_mark_node;
}
else if (decl_context == PARM || decl_context == CATCHPARM)
{
- error ("declaration of %qD as parameter", dname);
+ error_at (declarator->id_loc,
+ "declaration of %qD as parameter", dname);
return error_mark_node;
}
}
Index: testsuite/g++.dg/cpp0x/alias-decl-18.C
===
--- testsuite/g++.dg/cpp0x/alias-decl-18.C (revision 271542)
+++ testsuite/g++.dg/cpp0x/alias-decl-18.C (working copy)
@@ -5,5 +5,6 @@ template using ::T = void(int n); // {
template using operator int = void(int n); // { dg-error "" }
template using typename U = void; // { dg-error "" }
template using typename ::V = void(int n); // { dg-error "" }
-template using typename ::operator bool = void(int n); // {
dg-error "" }
+template using typename ::operator bool = void(int n); // {
dg-error "39:declaration" }
+// { dg-error "expected" "" { target *-*-* } .-1 }
using foo __attribute__((aligned(4)) = int; // { dg-error "" }
Index: testsuite/g++.dg/cpp0x/udlit-nofunc-neg.C
===
--- testsuite/g++.dg/cpp0x/udlit-nofunc-neg.C (revision 271542)
+++ testsuite/g++.dg/cpp0x/udlit-nofunc-neg.C (working copy)
@@ -3,7 +3,7 @@
// Test user-defined literals.
// Test error on non-function declaration.
-double operator"" _baddecl; // { dg-error "as non-function" }
+double operator"" _baddecl; // { dg-error "8:declaration of
.operator\"\"_baddecl. as non-function" }
template
- int operator"" _badtmpldecl; // { dg-error "as non-function" }
+ int operator"" _badtmpldecl; // { dg-error "7:declaration of
.operator\"\"_badtmpldecl. as non-function" }
Index: testsuite/g++.dg/parse/crash59.C
===
--- testsuite/g++.dg/parse/crash59.C(revision 271542)
+++ testsuite/g++.dg/parse/crash59.C(working copy)
@@ -1,3 +1,4 @@
// PR c++/53003
-struct A{ void a{} return b // { dg-error "function definition|expected" }
+struct A{ void a{} return b // { dg-error "16:function definition" }
+// { dg-error "expected" "" { target *-*-* } .-1 }
Index: testsuite/g++.dg/parse/e
Re: [C++ PATCH] Fix udlit-char-template-neg.C test
Hi, On 23/05/19 00:04, Marek Polacek wrote: On Thu, May 23, 2019 at 12:00:22AM +0200, Paolo Carlini wrote: Hi, On 22/05/19 23:45, Marek Polacek wrote: I noticed this test fails since r271492. Probably obvious, but... Tested on x86_64-linux, ok for trunk? I was wondering why I didn't notice it... It's because it only fails with check-c++-all not with check-c++. Anyway, it seems obvious to me too and the right line is the current one, good ;) I'll go ahead and check it in, thanks for confirming! Actually, Marek, there is a vey weird issue with our DejaGNU infrastructure which I noticed already in the past: dg-error "3:literal operator template|has invalid parameter list" is unexpectedly very weak: the column information, the 3, due to the | is mysteriously ignored. I think you want to shorten the string literal to simply "3:literal operator template", or alternately specify everything up to the "has invalid parameter list" (ie, see what I did for that patch and many other). Paolo.
Re: [C++ PATCH] Fix udlit-char-template-neg.C test
Hi, On 22/05/19 23:45, Marek Polacek wrote: I noticed this test fails since r271492. Probably obvious, but... Tested on x86_64-linux, ok for trunk? I was wondering why I didn't notice it... It's because it only fails with check-c++-all not with check-c++. Anyway, it seems obvious to me too and the right line is the current one, good ;) Paolo.
Re: [C++ Patch] Two literal operator template location fixes
On 22/05/19 10:28, Christophe Lyon wrote: On Tue, 21 May 2019 at 13:03, Paolo Carlini wrote: Hi, also in my back queue a few more location fixes (of course ;) Tested x86_64-linux. Thanks, Paolo. Hi, I think you incorrectly committed an additional chunk: I fixed that earlier today, sorry. Paolo.
Re: [C++ Patch] PR 67184 ("Missed optimization with C++11 final specifier")
Hi,
On 21/05/19 16:57, Jason Merrill wrote:
On 5/16/19 7:12 PM, Paolo Carlini wrote:
Hi,
when Roberto Agostino and I implemented the front-end
devirtualization of final overriders we missed this case, where it
comes from the base. It seems to me that by way of access_path the
existing approach can be neatly extended. Tested x86_64-linux.
+ || CLASSTYPE_FINAL (TREE_TYPE (cand->access_path)))
This will give the wrong type when the function is called with an
explicit scope; you probably want to look at argtype instead.
Yes, thanks, that works fine and is even neater. I'm finishing testing
the below. As you can see, I also added a line to final3.C where the two
versions of the call..c conditional give different answers. Note,
however, that in practice, in terms, say, of dumps, the difference is
difficult to emphasize because the call would not be virtual anyway (if
I'm not horribly mistaken ;)
Thanks, Paolo.
/
Index: cp/call.c
===
--- cp/call.c (revision 271459)
+++ cp/call.c (working copy)
@@ -8244,7 +8244,7 @@ build_over_call (struct z_candidate *cand, int fla
/* See if the function member or the whole class type is declared
final and the call can be devirtualized. */
if (DECL_FINAL_P (fn)
- || CLASSTYPE_FINAL (TYPE_METHOD_BASETYPE (TREE_TYPE (fn
+ || CLASSTYPE_FINAL (TREE_TYPE (argtype)))
flags |= LOOKUP_NONVIRTUAL;
/* [class.mfct.nonstatic]: If a nonstatic member function of a class
Index: testsuite/g++.dg/other/final3.C
===
--- testsuite/g++.dg/other/final3.C (nonexistent)
+++ testsuite/g++.dg/other/final3.C (working copy)
@@ -0,0 +1,27 @@
+// PR c++/67184
+// { dg-do compile { target c++11 } }
+// { dg-options "-fdump-tree-original" }
+
+struct V {
+ virtual void foo();
+};
+
+struct wV final : V {
+};
+
+struct oV final : V {
+ void foo();
+};
+
+void call(wV& x)
+{
+ x.foo();
+ x.V::foo();
+}
+
+void call(oV& x)
+{
+ x.foo();
+}
+
+// { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "original" } }
Index: testsuite/g++.dg/other/final4.C
===
--- testsuite/g++.dg/other/final4.C (nonexistent)
+++ testsuite/g++.dg/other/final4.C (working copy)
@@ -0,0 +1,16 @@
+// PR c++/67184
+// { dg-do compile { target c++11 } }
+// { dg-options "-fdump-tree-original" }
+
+struct B
+{
+ virtual void operator()();
+ virtual operator int();
+ virtual int operator++();
+};
+
+struct D final : B { };
+
+void foo(D& d) { d(); int t = d; ++d; }
+
+// { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "original" } }
Index: testsuite/g++.dg/other/final5.C
===
--- testsuite/g++.dg/other/final5.C (nonexistent)
+++ testsuite/g++.dg/other/final5.C (working copy)
@@ -0,0 +1,19 @@
+// PR c++/69445
+// { dg-do compile { target c++11 } }
+// { dg-options "-fdump-tree-original" }
+
+struct Base {
+ virtual void foo() const = 0;
+ virtual void bar() const {}
+};
+
+struct C final : Base {
+ void foo() const { }
+};
+
+void func(const C & c) {
+ c.bar();
+ c.foo();
+}
+
+// { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "original" } }
