<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div></div><div><br></div><div>-Terry</div><div><br></div><div><br><div><div>Am 15.05.2012 um 01:45 schrieb Richard Smith:</div><br class="Apple-interchange-newline"><blockquote type="cite">Can you add the word 'arguments' to the end of these diagnostics:<div><br></div><div><div>+ test18_a(b, b); // expected-error {{too many arguments to function call, expected single argument 'a', have 2}}</div>
<div><div>+ j(2, 3); // expected-error{{too many arguments to function call, expected at most single argument 'f', have 2}}</div></div><div><br></div><div>Other than that, I think this is fine.</div><br><div class="gmail_quote">
On Mon, May 14, 2012 at 10:36 PM, Terry Long <span dir="ltr"><<a href="mailto:macmantrl@me.com" target="_blank">macmantrl@me.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Any updates on if my latest patch incorporating Jordy's suggestions is acceptable and ready for commit by someone?<br>
Attached again is the patch for reference.<br>
<br>
Thanks,<br>
<br>
Terry<br>
<br>
<br>
<br><br>
Am 11.05.2012 um 15:49 schrieb Terry Long:<br>
<br>
> Here's another patch which implements all of the suggested changes.<br>
><br>
> -Terry<br>
><br>
> <PR11857v3.patch><br>
> Am 11.05.2012 um 13:16 schrieb Terry Long:<br>
><br>
>>> 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):<br>
>>><br>
>>>> too few arguments to function call, [single] argument 'a' was not specified<br>
>>><br>
>><br>
>> I agree, using 'single' helps clarify that the function takes only one argument.<br>
>><br>
>>><br>
>>> 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).<br>
>>><br>
>>>> candidate function not viable: requires [single] argument 'n', but 2 [arguments] were provided<br>
>>><br>
>>> 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.<br>
>><br>
>> 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'.<br>
>><br>
>>><br>
>>>> candidate function not viable: requires[*] at most argument 'n', but 2 [arguments] were provided<br>
>>><br>
>>><br>
>>> * 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.<br>
>><br>
>> Yes, allows is more fitting. A function with 1 optional argument is not requiring anything.<br>
>><br>
>>><br>
>>>> candidate function not viable: requires at least argument 'n', but none[*] were provided<br>
>>><br>
>>> s/none/no arguments/, same as above.<br>
>>><br>
>>> Also, why no version for err_typecheck_call_too_many_args, since the overload resolution gets one for too many args?<br>
>><br>
>> 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.<br>
>><br>
>>><br>
>>>> too many arguments to function call, expected single argument 'n', have 2 [arguments]<br>
>>><br>
>>> Here I could go either way on including the last "arguments", since it was already stated at the beginning.<br>
>><br>
>> 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.<br>
>><br>
>>><br>
>>> What do you think?<br>
>><br>
>> I will create a new patch with these changes unless anyone else objects.<br>
>><br>
>><br>
>> -Terry<br>
>><br>
>><br>
>>> Jordy<br>
>>><br>
>>><br>
>>> On May 11, 2012, at 1:18, Richard Smith wrote:<br>
>>><br>
>>>> Great, thanks for working on this! Committed as r156607.<br>
>>>><br>
>>>> On Thu, May 10, 2012 at 7:31 PM, Terry Long <<a href="mailto:macmantrl@me.com">macmantrl@me.com</a>> wrote:<br>
>>>> I've added more test coverage, removed deprecated methods, and extended the enhancement to the 'candidate function not viable' diagnostic for C++.<br>
>>>><br>
>>>> Patch version 2 attached.<br>
>>>><br>
>>>> -Terry Long<br>
>>>><br>
>>>><br>
>>>><br>
>>>><br>
>>>> Am 10.05.2012 um 19:17 schrieb Richard Smith:<br>
>>>><br>
>>>>> On Thu, May 10, 2012 at 4:00 PM, Terry Long <<a href="mailto:macmantrl@me.com">macmantrl@me.com</a>> wrote:<br>
>>>>>> The patch generally looks good, thanks!<br>
>>>>><br>
>>>>> Great, thanks for the feedback.<br>
>>>>><br>
>>>>>> Presumably this only applies to the case where there are no arguments, because otherwise we couldn't know /which/ argument was missing?<br>
>>>>><br>
>>>>> 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.<br>
>>>>><br>
>>>>>> 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().<br>
>>>>><br>
>>>>> 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?<br>
>>>>><br>
>>>>> It's in include/clang/AST/Decl.h:138-141, though for some reason those comments aren't exposed to doxygen...<br>
>>>>><br>
>>>>>> It would also be great to extend this to the 'candidate function not viable' diagnostics in C++.<br>
>>>>><br>
>>>>> I can take a look at this too.<br>
>>>>><br>
>>>>> Awesome, thanks.<br>
>>>><br>
>>>><br>
>>>><br>
>>>> _______________________________________________<br>
>>>> cfe-commits mailing list<br>
>>>> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
>>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
>>><br>
>><br>
>> _______________________________________________<br>
>> cfe-commits mailing list<br>
>> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br>
<br></blockquote></div><br></div>
</blockquote></div><br></div></body></html>