On 2/6/25 3:05 PM, Simon Martin wrote:
Hi Jason,
On 6 Feb 2025, at 16:48, Jason Merrill wrote:
On 2/5/25 2:21 PM, Simon Martin wrote:
Hi Jason,
On 4 Feb 2025, at 21:23, Jason Merrill wrote:
On 2/4/25 3:03 PM, Jason Merrill wrote:
On 2/4/25 11:45 AM, Simon Martin wrote:
On 4 Feb 2025, at 17:17, Jason Merrill wrote:
On 2/4/25 10:56 AM, Simon Martin wrote:
Hi Jason,
On 4 Feb 2025, at 16:39, Jason Merrill wrote:
On 1/15/25 9:56 AM, Jason Merrill wrote:
On 1/15/25 7:24 AM, Simon Martin wrote:
Hi,
On 14 Jan 2025, at 23:31, Jason Merrill wrote:
On 1/14/25 2:13 PM, Simon Martin wrote:
On 10 Jan 2025, at 19:10, Andrew Pinski wrote:
On Fri, Jan 10, 2025 at 3:18 AM Simon Martin
<si...@nasilyan.com>
wrote:
We currently accept the following invalid code (EDG and
MSVC
do
as
well)
clang does too:
https://github.com/llvm/llvm-project/issues/121706 .
Note it might be useful if a testcase with multiply `*` is
included
too:
```
struct A {
****A ();
};
```
Thanks, makes sense to add those. Done in the attached
updated
revision,
successfully tested on x86_64-pc-linux-gnu.
+/* Check that it's OK to declare a function at ID_LOC with
the
indicated TYPE,
+ TYPE_QUALS and DECLARATOR. SFK indicates the kind
of
special
function (if
+ any) that this function is. OPTYPE is the type
given
in
a
conversion
operator declaration, or the class type for a
constructor/destructor.
Returns the actual return type of the
function;
that
may
be
different
than TYPE if an error occurs, or for certain
special
functions.
*/
@@ -12361,8 +12362,19 @@ check_special_function_return_type
(special_function_kind sfk,
tree type,
tree optype,
int
type_quals,
+ const cp_declarator
*declarator,
+ location_t id_loc,
id_loc should be the same as declarator->id_loc?
You’re right.
const
location_t*
locations)
{
+ /* If TYPE is unspecified, DECLARATOR, if set, should
not
represent a pointer
+ or a reference type. */
+ if (type == NULL_TREE
+ && declarator
+ && (declarator->kind == cdk_pointer
+ || declarator->kind == cdk_reference))
+ error_at (id_loc, "expected unqualified-id before
%qs
token",
+ declarator->kind == cdk_pointer ? "*" :
"&");
...and id_loc isn't the location of the ptr-operator, it's
the
location of the identifier, so this indicates the wrong
column.
I
think using declarator->id_loc makes sense, just not
pretending
it's
the location of the *.
Good catch, thanks.
Let's give diagnostics more like the others later in the
function
instead of trying to emulate cp_parser_error.
Makes sense. This is what the updated patch does,
successfully
tested on
x86_64-pc-linux-gnu. OK for GCC 16?
OK.
Does this also fix 118304? If so, let's go ahead and apply it
to
GCC
15.
I have checked just now, and we still ICE for 118304’s
testcase
with
that fix.
Why doesn't the preeexisting
type = void_type_node;
in check_special_function_return_type fix the return type and
avoid
the ICE?
We hit the gcc_assert at method.cc:3593, that Marek’s fix
bypasses.
Yes, but why doesn't check_special_function_return_type prevent
that?
Ah, because we call it before walking the declarator. We need to
check again later, perhaps in grokfndecl, that the type is correct.
Perhaps instead of your patch.
One “issue” with adding another check in or close to grokfndecl
is
that DECLARATOR will have “been moved to the ID”, and the fact
that
we had a CDK_POINTER kind is “lost”. We could obviously somehow
propagate this information, but there might be something easier.
The information isn't lost: it's now reflected in the (wrong) return
type. One place it would make sense to check would be
if (ctype && (sfk == sfk_constructor
|| sfk == sfk_destructor))
{
/* We are within a class's scope. If our declarator
name
is the same as the class name, and we are defining
a
function, then it is a constructor/destructor, and
therefore
returns a void type. */
Here 'type' is still the return type, we haven't gotten to
build_function_type yet.
That’s true. However, doesn’t it make sense to cram all the checks
about the return type of special functions in
check_special_function_return_type, and return an error if that return
type is invalid?
This error seems easily recoverable since we know what the type needs to
be, there's no need for error return from grokdeclarator.
However, an alternative to my suggestion above would be to build on your
patch by making check_special_function_return_type actually strip the
invalid declarators, not just complain about them.
Jason
