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

[Alexander Lanin via Phabricator via llvm-commits]

AlexanderLanin marked 2 inline comments as done and an inline comment as not done.
AlexanderLanin added inline comments.


================
Comment at: clang/test/Sema/warn-documentation-fixits.cpp:149
+  */
+void run(const char *const arg[], int argc);
+
----------------
AlexanderLanin wrote:
> Mordante wrote:
> > Here I would expect the `cnt` to be matched with `argc`.
> This is currently not detected. Here is the minimal scenario:
> ```
> /// \param a This one is correctly documented.
> /// \param c Total mismatch. Fix it since it's only one comment to only one parameter ?!
> int oneUnmatchedCommentToOneParameter(int a, int b);
> ```
> 
> Unfortunately I disagree. Sorry. Auto fixing that would be a no go for me personally. Because in my personal experience it's a quite common use case that one parameter is removed (but not the documentation) and then a different *unrelated* parameter is introduced (with no documentation). Just moving the old documentation over to this new parameter would hide/create a significant problem. Without 'fixing' it's at least obvious that there is a problem there.
> I've not encountered this problem in real life with only one parameter, so I'm ok with `fixSinceThereIsOnlyOneOption` behavior.
Ok, that's not what is implemented: It does get fixed. So I was contemplating on what that means. Any code change, including or especially the automatic ones, do go through a code review. So it 'should' be ok to 'fix' stuff. That's what we have reviews for anyway.
I'm not too happy about it due to reasons above, but ok.

In this case it doesn't get suggested/fixed immediately on the first pass. Only after arg[] is fixed to arg it will be detected on subsequent compilation. It's not ideal, but good enough?!


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

https://reviews.llvm.org/D74877