Compiler output: ``` 24 | static_assert(std::is_same<int, long>::value,"eee"); | ^~~ ``` Range mark only `std` instead of whole expression. At least is should mark `value`.
btw how reduce "Importance" of this bug? Right now it have same level as bug that could break my code.
(In reply to trashyankes from comment #1) > btw how reduce "Importance" of this bug? You don't. That field doesn't matter.
Proper testcase that actually compiles: #include <type_traits> static_assert(std::is_same<int, long>::value,"eee"); 87386.cc:2:15: error: static assertion failed: eee 2 | static_assert(std::is_same<int, long>::value,"eee"); | Reduced testcase without header dependencies: namespace foo { template<typename> struct test { static constexpr bool value = false; }; } static_assert(foo::test<int>::value, "eee"); 87386.cc:4:15: error: static assertion failed: eee 4 | static_assert(foo::test<int>::value, "eee"); | ^~~ This is a regression, as GCC 7 highlighted the keyword 'static_assert' instead, which makes more sense than just the first token of the condition: 87386.cc:4:1: error: static assertion failed: eee static_assert(foo::test<int>::value, "eee"); ^~~~~~~~~~~~~
Started with r255255 C++: improve location of static_assert errors gcc/cp/ChangeLog: * parser.c (cp_parser_unary_expression): Generate a location for "noexcept". (cp_parser_trait_expr): Generate and return a location_t, converting the return type from tree to cp_expr. (cp_parser_static_assert): Pass location of the condition to finish_static_assert, rather than that of the "static_assert" token, where available.
It would be better to highlight the whole condition: 87386.cc:4:15: error: static assertion failed: eee 4 | static_assert(foo::test<int>::value, "eee"); | ^~~~~~~~~~~~~~~~~~~~~ Which is what already happens for a more complex expression: namespace foo { template<typename> struct test { static constexpr bool value = false; }; } static_assert(foo::test<int>::value && true, ""); 87386.cc:4:37: error: static assertion failed 4 | static_assert(foo::test<int>::value && true, ""); | ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
With: --- gcc/cp/parser.c.jj 2018-11-20 08:41:28.686923718 +0100 +++ gcc/cp/parser.c 2018-11-20 17:06:27.942626189 +0100 @@ -5602,7 +5602,7 @@ cp_parser_primary_expression (cp_parser /*is_namespace=*/false, /*check_dependency=*/true, &ambiguous_decls, - id_expr_token->location); + id_expression.get_location ()); /* If the lookup was ambiguous, an error will already have been issued. */ if (ambiguous_decls) @@ -5673,7 +5673,7 @@ cp_parser_primary_expression (cp_parser if (parser->local_variables_forbidden_p && local_variable_p (decl)) { - error_at (id_expr_token->location, + error_at (id_expression.get_location (), "local variable %qD may not appear in this context", decl.get_value ()); return error_mark_node; @@ -5692,7 +5692,7 @@ cp_parser_primary_expression (cp_parser id_expression.get_location ())); if (error_msg) cp_parser_error (parser, error_msg); - decl.set_location (id_expr_token->location); + decl.set_location (id_expression.get_location ()); return decl; } @@ -5758,6 +5758,7 @@ cp_parser_id_expression (cp_parser *pars { bool global_scope_p; bool nested_name_specifier_p; + location_t start_loc = cp_lexer_peek_token (parser->lexer)->location; /* Assume the `template' keyword was not used. */ if (template_p) @@ -5809,6 +5810,9 @@ cp_parser_id_expression (cp_parser *pars parser->object_scope = saved_object_scope; parser->qualifying_scope = saved_qualifying_scope; + location_t loc = make_location (unqualified_id.get_start (), start_loc, + unqualified_id.get_finish ()); + unqualified_id.set_location (loc); return unqualified_id; } /* Otherwise, if we are in global scope, then we are looking at one the output is: pr87386.C:4:31: error: static assertion failed: foo 4 | static_assert(foo::test<int>::value, "foo"); | ~~~~~~~~~~~~~~~~^~~~~ pr87386.C:5:37: error: static assertion failed: bar 5 | static_assert(foo::test<int>::value && true, "bar"); | ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~ Is that what we want? I.e. put the care at the start of the identifier in the id-expression and use range of the whole id-expression? We'd need to adjust probably g++.dg/cpp0x/auto52.C g++.dg/cpp0x/pr51420.C g++.dg/cpp0x/udlit-declare-neg.C g++.dg/lookup/suggestions2.C g++.dg/parse/error17.C g++.dg/spellcheck-pr77829.C g++.dg/spellcheck-pr78656.C g++.dg/spellcheck-pr79298.C g++.dg/spellcheck-single-vs-multiple.C testcases, so before I do that I'd like to get agreement if this is the right thing to do. David?
Created attachment 45046 [details] gcc9-pr87386.patch More complete patch. Though, in some cases it might be better to just use range of just the last identifier of an id-expression, and the suggestions look weird too. foo::bar ~~~~~^~~ baz instead of baz David?
Author: jakub Date: Wed Nov 21 22:41:07 2018 New Revision: 266359 URL: https://gcc.gnu.org/viewcvs?rev=266359&root=gcc&view=rev Log: PR c++/87386 * parser.c (cp_parser_primary_expression): Use id_expression.get_location () instead of id_expr_token->location. Adjust the range from id_expr_token->location to id_expressio.get_finish (). (cp_parser_operator_function_id): Pass location of the operator token down to cp_parser_operator. (cp_parser_operator): Add start_loc argument, always construct a location with caret at start_loc and range from start_loc to the finish of the last token. gcc/testsuite/ * g++.dg/diagnostic/pr87386.C: New test. * g++.dg/parse/error17.C: Adjust expected diagnostics. libstdc++-v3/ * testsuite/20_util/scoped_allocator/69293_neg.cc: Adjust expected line. * testsuite/20_util/uses_allocator/cons_neg.cc: Likewise. * testsuite/20_util/uses_allocator/69293_neg.cc: Likewise. * testsuite/experimental/propagate_const/requirements2.cc: Likewise. * testsuite/experimental/propagate_const/requirements3.cc: Likewise. * testsuite/experimental/propagate_const/requirements4.cc: Likewise. * testsuite/experimental/propagate_const/requirements5.cc: Likewise. Added: trunk/gcc/testsuite/g++.dg/diagnostic/pr87386.C Modified: trunk/gcc/cp/ChangeLog trunk/gcc/cp/parser.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/g++.dg/parse/error17.C trunk/libstdc++-v3/ChangeLog trunk/libstdc++-v3/testsuite/20_util/scoped_allocator/69293_neg.cc trunk/libstdc++-v3/testsuite/20_util/uses_allocator/69293_neg.cc trunk/libstdc++-v3/testsuite/20_util/uses_allocator/cons_neg.cc trunk/libstdc++-v3/testsuite/experimental/propagate_const/requirements2.cc trunk/libstdc++-v3/testsuite/experimental/propagate_const/requirements3.cc trunk/libstdc++-v3/testsuite/experimental/propagate_const/requirements4.cc trunk/libstdc++-v3/testsuite/experimental/propagate_const/requirements5.cc
Author: jakub Date: Thu Nov 22 09:26:29 2018 New Revision: 266369 URL: https://gcc.gnu.org/viewcvs?rev=266369&root=gcc&view=rev Log: PR c++/87386 * parser.c (cp_parser_operator): Use str.get_value () instead of just str in USERDEF_LITERAL_VALUE and USERDEF_LITERAL_SUFFIX_ID arguments. Modified: trunk/gcc/cp/ChangeLog trunk/gcc/cp/parser.c
Fixed.
Not going to backport for 8.x.