[PATCH] D74877: [clang] fix incorrect Wdocumentation fix-its

Alexander Lanin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Mar 21 12:15:49 PDT 2020


AlexanderLanin added a comment.

Thanks for the feedback!!

> Do you want to continue this patch or switch to the other one as you mentioned before?

I had already updated this pull request to incorporate improvements from the github link. Sorry I didn't make that clear.
It's doesn't have those very small functions. That is in order to keep most of the not so easy understandable mapping vectors in small local scope.
Also to avoid some redundancy like adding all mappings and then removing the duplicates.

> In the tests please include the exact issues mentioned in PR43755 and PR43808.

Both reports are from me and I'm quite certain I have them covered in tests. I will add a reference to the exact test cases.

> Take all \params and function parameters in account when looking for a solution and find the best total solution.

Currently all unmapped parameters are taken into account.
I cannot imagine a use case where taking the mapped ones into account could influence the outcome.
Also I personally don't like breaking any existing correct documentation.
Do you have some example?

> If the edit distance is too large don't use that suggestion.

Shall I remove the 1:1 exception which disregards edit distance?
The one from fixSinceThereIsOnlyOneOption?
I tried to not modify any not bug related behavior and this was there before.

> Since most people tend to document \params in the same order as the function parameters you can also take that into account.

Sounds reasonable, but it might not be trivial to find an appropriate heuristic weighting this against edit distance.
I would like to exclude it from this patch and attempt that in the next one.

> Can you add a few more complex test cases, for example something like:

Sure! I missed that one. I'll add some more.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74877/new/

https://reviews.llvm.org/D74877





More information about the llvm-commits mailing list