-Terry
Am 15.05.2012 um 01:45 schrieb Richard Smith: Can you add the word 'arguments' to the end of these diagnostics:
+ test18_a(b, b); // expected-error {{too many arguments to function call, expected single argument 'a', have 2}}
+ j(2, 3); // expected-error{{too many arguments to function call, expected at most single argument 'f', have 2}}
Other than that, I think this is fine.
On Mon, May 14, 2012 at 10:36 PM, Terry Long <[email protected]> wrote:
Any updates on if my latest patch incorporating Jordy's suggestions is acceptable and ready for commit by someone?
Attached again is the patch for reference.
Thanks,
Terry
Am 11.05.2012 um 15:49 schrieb Terry Long:
> Here's another patch which implements all of the suggested changes.
>
> -Terry
>
> <PR11857v3.patch>
> Am 11.05.2012 um 13:16 schrieb Terry Long:
>
>>> Sorry for weighing in so late, but some of the messages don't seem quite right to me. A "single" could help a lot for some of these cases (suggested fixes in brackets):
>>>
>>>> too few arguments to function call, [single] argument 'a' was not specified
>>>
>>
>> I agree, using 'single' helps clarify that the function takes only one argument.
>>
>>>
>>> Without the "single" I feel like this is a warning for /any/ "too few args" situation that's only missing one arg (e.g. 2 for 3).
>>>
>>>> candidate function not viable: requires [single] argument 'n', but 2 [arguments] were provided
>>>
>>> This doesn't feel like valid English as written. Two whats? ("I was going to go to the state of Hawaii, but I went to two instead.") And here the "single" really underscores that the problem is too many arguments.
>>
>> I originally had 'arguments' in the message, but took it out assuming "argument 'n'" was enough to know we were talking about arguments. Given your Hawaii example, I can see that the more correct way is probably to explicitly say 'arguments'.
>>
>>>
>>>> candidate function not viable: requires[*] at most argument 'n', but 2 [arguments] were provided
>>>
>>>
>>> * s/requires/allows? In this specific case of "0 or 1" it seems more fitting; not sure about the other "at most" warnings. Also "arguments", same as above.
>>
>> Yes, allows is more fitting. A function with 1 optional argument is not requiring anything.
>>
>>>
>>>> candidate function not viable: requires at least argument 'n', but none[*] were provided
>>>
>>> s/none/no arguments/, same as above.
>>>
>>> Also, why no version for err_typecheck_call_too_many_args, since the overload resolution gets one for too many args?
>>
>> I overlooked this on my first attempt at the patch, and only added it to the 'candidate not viable' diagnostic because the same diagnostic was being used for /at most/at least/exactly/. I will definitely add this in.
>>
>>>
>>>> too many arguments to function call, expected single argument 'n', have 2 [arguments]
>>>
>>> Here I could go either way on including the last "arguments", since it was already stated at the beginning.
>>
>> I would personally leave out 'arguments' to be consistent with all the other cases which use 'expected 2, have 0' form, and it is implied by the beginning phrase.
>>
>>>
>>> What do you think?
>>
>> I will create a new patch with these changes unless anyone else objects.
>>
>>
>> -Terry
>>
>>
>>> Jordy
>>>
>>>
>>> On May 11, 2012, at 1:18, Richard Smith wrote:
>>>
>>>> Great, thanks for working on this! Committed as r156607.
>>>>
>>>> On Thu, May 10, 2012 at 7:31 PM, Terry Long <[email protected]> wrote:
>>>> I've added more test coverage, removed deprecated methods, and extended the enhancement to the 'candidate function not viable' diagnostic for C++.
>>>>
>>>> Patch version 2 attached.
>>>>
>>>> -Terry Long
>>>>
>>>>
>>>>
>>>>
>>>> Am 10.05.2012 um 19:17 schrieb Richard Smith:
>>>>
>>>>> On Thu, May 10, 2012 at 4:00 PM, Terry Long <[email protected]> wrote:
>>>>>> The patch generally looks good, thanks!
>>>>>
>>>>> Great, thanks for the feedback.
>>>>>
>>>>>> Presumably this only applies to the case where there are no arguments, because otherwise we couldn't know /which/ argument was missing?
>>>>>
>>>>> Yes, only for the case where there are no arguments to a function that takes 1 argument. Almost impossible to determine the missing argument(s) otherwise.
>>>>>
>>>>>> Please add test coverage for the err_typecheck_call_too_few_args_at_least_one diagnostic. Also, NamedDecl::getNameAsString is deprecated; please just use "<< FDecl->getParamDecl(0)", and use getParamDecl(0)->getDeclName()'s operator bool() in the test, rather than empty().
>>>>>
>>>>> OK, I'll update this. I was using the online doxygen docs and didn't see any deprecation warnings. Anywhere where I can find that information?
>>>>>
>>>>> It's in include/clang/AST/Decl.h:138-141, though for some reason those comments aren't exposed to doxygen...
>>>>>
>>>>>> It would also be great to extend this to the 'candidate function not viable' diagnostics in C++.
>>>>>
>>>>> I can take a look at this too.
>>>>>
>>>>> Awesome, thanks.
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> [email protected]
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> [email protected]
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
> _______________________________________________
> cfe-commits mailing list
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
|