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. Do you want me to merge the fix for 118306 to trunk, so that Marek can then merge his fix without a gcc_assert (seen_error ()) instead of the current gcc_assert (true || seen_error ())?
Simon
