[clang] fix incorrect Wdocumentation fix-its

[AlexanderLanin]

Summary

Wdocumentation no longer provides fix-its which map multiple param comments to the same parameter.

This commit fixes:

• https://bugs.llvm.org/show_bug.cgi?id=43755
• https://bugs.llvm.org/show_bug.cgi?id=43808

[Mordante]

Thanks for your patch. I also would like to get better fixits for the arguments.

I would also like to hear @gribozavr2's input.

clang/lib/AST/CommentSema.cpp

739

I don't feel the name Mapped conveys what the variable does. IMO Corrections would be a better name.

752

Please don't use curly braces for single line if and else statements (also at other locations).
It makes no sense to test this condition every iteration. Please move this test out of the loop.

763

I don't like adding duplicates here, and removed them in setDuplicatesToNullptr. This is inefficient.
Maybe add a mapping between CorrectedParamIndex and the first position in Mapped. Then when a duplicate is found the first position can be set to nullptr and the new position not added.

929–930

Please capitalize mapping.
I don't feel the name mapping conveys what the variable does. IMO Corrections would be a better name.
Please don't use auto when the type is not clear.

clang/test/Sema/warn-documentation-fixits.cpp

37

worthen -> worsen

llvm/unittests/ADT/StringRefTest.cpp

539

I don't mind this, but was it intentionally?

[AlexanderLanin]

clang/lib/AST/CommentSema.cpp

752

I didn't touch this section that much. I extracted one of the ifs from the loop, but apparantely not the other... anyway I can certainly fix this.

763

I fully agree on that. However I went too far with cleanup and now I'm not quite sure what to do. To avoid adding another parallel review here, I'll just point you to https://github.com/AlexanderLanin/llvm-project/commit/d043f49089ebe3b0d1b4d8df02fbd0b3985b4d18 if thats fine? Just to figure out whether to abandon this change as it is here and switch to the other one or not. I ended up reducing resolveParamCommandIndexes to less then a dozen (hopefully readable) lines. Adding and removing is resolved with a MappedCounter, since other approaches failed to recognize triplicates.
If that looks fine in general I would replace this review. I personally like the smaller understandable functional methods, if that doesn't violate the general rules/ideas where the code should go. Otherwise I'll apply your findings here.

llvm/unittests/ADT/StringRefTest.cpp

539

I know it's slightly off topic, but not quite worth it's own commit. I was trying to figure out why 'ThisOneHasBeenRemovedButIsStillDocumented' was auto fixed to 'aaa' and my first guess was edit_distance doesn't correctly handle removed chars. As the code itself is not really readable for mere mortals and such a test was missing I added it here.

Turns out it was a special rule for if only one \\param, then don't check edit_distance. This is now slightly modified into if only one \\param and only one ParmVarDecl, then don't check edit_distance

[Mordante]

clang/lib/AST/CommentSema.cpp

763

I think it makes sense to abandon this review and start a new one. As far as I know we don't have a policy regarding the size of functions. Personally I prefer smaller functions. Before putting up the new review, please address the comments in this review. Please have a look at coding standard https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable regarding the usage of auto.

I'm still not really fond of the algorithm used to remove the duplicates. I'll give it some thought to see whether I can come up with a better suggestion.

llvm/unittests/ADT/StringRefTest.cpp

539

Oke, just wanted to make sure it was intentionally.

[AlexanderLanin]

Slightly more code changed, assuming it's better understandable now. Review findings incorporated.

[Mordante]

@gribozavr2 what's your opinion of this patch?

[AlexanderLanin]

*ping*

[Mordante]

Sorry it took a while.

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

Some other remarks:

• In the tests please include the exact issues mentioned in PR43755 and PR43808.
• I'd also like the patch to be a bit more robust regarding the fix-its. @gribozavr2 also offered some suggestions:
  • Take all \params and function parameters in account when looking for a solution and find the best total solution.
  • If the edit distance is too large don't use that suggestion.
  • Since most people tend to document \params in the same order as the function parameters you can also take that into account.

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

/**
 * \param aabb X
 * \param abbb Y
 */
void foo(int NotMe, int bbbb);

Which comment will be attached to bbbb?

[AlexanderLanin]

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.

[AlexanderLanin]

More tests added to prevent further regressions. It's quite difficult to figure out tests which might fail, but now there is a whole bunch of them so it looks good.
Added explicit references from tests to the two bug reports.

If there is anything else I should fix, I'd gladly do so!

If there is nothing else please commit it as I cannot.
Alexander Lanin <[e-mail]>

[AlexanderLanin]

*ping*

[Mordante]

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?

For example what happens in this case:

/**
 * \param aaaa Should match aaa
 * \param aaab  Should match aab
 * \param aaac  Should match aac
 */
void foo(int aaa, int aab, int aac);

And in the almost similar case:

/**
 * \param aaaa Should match aaa
 * \param aaab  Should match aab
 * \param aaac  Should match aac
 */
void foo(int aac, int aab, int aaa);

Does it give the same final result or does the order of the comments and parameters influence the outcome?

I would like to have a closer look at the algorithm you're using, I hope to find time for that later this week.

clang/lib/AST/CommentSema.cpp

738

Please capitalize the i and e.

787

Since the auto type is a pointer, please use a * to make it clear.

793

Please don't use auto unless the type is clear.

798

Is it intended that the assignment ParmVarDeclToParamCommandComment[PCC->getParamIndex()] = PCC; is executed? If so please add some comment.

814

Please capitalize the i and e.

824

Since the auto type is a pointer, please use a * to make it clear.

854

Please capitalize the i and e.

897

Please capitalize the i and e.

clang/test/Sema/warn-documentation-fixits.cpp

76

Why use a different test case than in the bug report? I really would like to see this patch fixes that case.

[AlexanderLanin]

I need to have a look why the mentioned test case is failing. I'll also add the two examples you provided to the next upload.

clang/lib/AST/CommentSema.cpp

738

Sorry such stuff should not require a manual review. I fixed this and similar issues by running clang-tidy --checks="readability-identifier-naming" -p=/home/alex/llvm/build CommentSema.cpp --fix

clang/test/Sema/warn-documentation-fixits.cpp

76

You are totally right. Added test case; It's failing; needs analysis. I thought I had that fixed. Will check ASAP.

[AlexanderLanin]

• Add test cases exactly as in bug reports.
• Test case from https://bugs.llvm.org/show_bug.cgi?id=43808 triggered a second bug due to the short parameter names used. Removed incorrect quick exit in SimpleTypoCorrector (MinPossibleEditDistance).
• The two requested cases don't work since 'aaab' is as close to 'aaa' as it is to 'aab' (one char). Nothing is changed as there is no best match available. Instead two new test cases with aaab, aaac and aaad added (test3to3_allAlmost + test3to3_allAlmost_rev).

[AlexanderLanin]

*ping*

[AlexanderLanin]

*ping*

[AlexanderLanin]

*ping*
Last change was over 3 months ago. Should this be merged or abandoned?
It doesn't solve everything. It doesn't improve everything. But it's a fix to two specific bugs.
Is someone missing as a reviewer to get this accepted/rejected any faster?

[AlexanderLanin]

Rebased and stored at https://github.com/AlexanderLanin/llvm-project/tree/Wdocumentation-fix-its so I have a branch next time I need to rebase :-)

Still looking for someone to approve and to commit this change.

[AlexanderLanin]

Added one missing expect statement in this revision.

Could someone please review and optionally approve and to commit this change?
That would be great!
It's been open for a while now ;-)

[Mordante]

I think the patch looks better, but I still would like to get @gribozavr2 's opinion.

I'm not convinced the new approach is better in all cases, so I'm somewhat concerned about regressions from the current behaviour. Did you test this patch on a codebase to see the difference between the previous and new behaviour?

For the next time when providing a patch, please do the cleanups in a separate patch, that makes reviewing the real changes a bit easier.

clang/lib/AST/CommentSema.cpp

22

Is the debug output really that interesting after the code has been committed?

756

In general Clang doesn't use top level const often, please remove them from the function argument.

759

Please use auto * in range-based for loops.

760

Please use auto * since the type is already on the right hand side.

779

I would suggest to rename the function. The current name doesn't match what the function does. For example findUndocumentedParams.

787

I would suggest to make the name of the variable it bit shorter to improve readability.

790

Please use const auto *

828

Please use const auto*

859

Please reduce the number of blank lines in the hunk below.

873

I'm not sure whether that's the best behaviour. Can't we test which of the two suggestions is the best and then keep the suggestion there?

917

Please don't use braces on if's with one substatement. Maybe move the comment before the if.

clang/test/Sema/warn-documentation-fixits.cpp

81

Here I actually would like to get the fixit. No longer showing the argc when typing ArgC feels not good to me.

91

Here there's one undocumented parameter left and one \param why not match them?

96

why test for ZZZZZZ ?

137

Please reduce the amount of whitespace.

149

Here I would expect the cnt to be matched with argc.

[AlexanderLanin]

clang/lib/AST/CommentSema.cpp

22

There are a few debug print statements left in code below. They are certainly not interesting until someone tries to debug the code ;-)
Shall I remove this line and those llvm::dbgs() statements?

756

Sorry I was not familiar with that convention. I've removed const which affects only the definition and not the declaration (e.g. I left in case of const ref).

759

Sure, I'll fix it! It even felt a little strange to me. I tried to follow the guidelines which say to skip the type if it's already obvious from context. I though the exact type is not obvious since it's not getBlockContentComment(). But it's close enough :-)

779

Thanks, that should have been obvious from the comment 'Returns all ParmVarDecl without a comment'.

873

Sounds like a very reasonable improvement idea.
However on first glance it would require quite some changes starting with SimpleTypoCorrector/correctTypoInParmVarReference even returning the BestEditDistance.
Would it make more sense to address them in a follow up patch?

clang/test/Sema/warn-documentation-fixits.cpp

81

They both have the same edit distance of two mismatching chars. There is no logic in StringRef::edit_distance which would say 'c' is closer to 'C' then it is to 'V'.
I guess that would be a nice follow up improvement e.g. via another flag to edit_distance bool CaseIsNotVeryImportant.
edit_distance is used by ~23 files and I guess several of them could benefit from that new flag.

Note: at the moment both are fixed to 'argc' which is worse then not fixing at all.

96

Copy paste error. There is a ZZZZZZ before and after this test case.

149

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.

[AlexanderLanin]

clang/test/Sema/warn-documentation-fixits.cpp

149

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?!

[AlexanderLanin]

I'm not convinced the new approach is better in all cases, so I'm somewhat concerned about regressions from the current behaviour. Did you test this patch on a codebase to see the difference between the previous and new behaviour?

There are no warnings in llvm or ccache, all I found is a low quality codebase at work which does produce 1007 unique Wdocumentation warnings (see wc command).

It *seems* I can run this via:

mkdir -p build-clang-D74877 && cd build-clang-D74877
CC=/home/alex/llvm/build-D74877/bin/clang CXX=/home/alex/llvm/build-D74877/bin/clang++ cmake .. -DCMAKE_CXX_FLAGS="-Wdocumentation -Wno-documentation-deprecated-sync" -GNinja
CC=/home/alex/llvm/build-D74877/bin/clang CXX=/home/alex/llvm/build-D74877/bin/clang++ cmake .. -DCMAKE_CXX_FLAGS="-Wdocumentation -Wno-documentation-deprecated-sync -fsyntax-only" -GNinja
ninja -j1 -k10000 > warnings.txt
grep "\[-Wdocumentation\]" warnings.txt | grep -v "empty paragraph passed to" | grep -v "^\[" | sort | uniq | wc -l
cd ..

mkdir -p build-clang-master && cd build-clang-master
CC=/home/alex/llvm/build-master/bin/clang CXX=/home/alex/llvm/build-master/bin/clang++ cmake .. -DCMAKE_CXX_FLAGS="-Wdocumentation -Wno-documentation-deprecated-sync" -GNinja
CC=/home/alex/llvm/build-master/bin/clang CXX=/home/alex/llvm/build-master/bin/clang++ cmake .. -DCMAKE_CXX_FLAGS="-Wdocumentation -Wno-documentation-deprecated-sync -fsyntax-only" -GNinja
ninja -j1 -k10000 > warnings.txt
grep "\[-Wdocumentation\]" warnings.txt | grep -v "empty paragraph passed to" | grep -v "^\[" | sort | uniq | wc -l
cd ..

# .. in order to leave git
grep -v "^\[" build-clang-master/warnings.txt > ../clang-master-warnings.txt
grep -v "^\[" build-clang-D74877/warnings.txt > ../clang-D74877-warnings.txt

git diff ../clang-master-warnings.txt ../clang-D74877-warnings.txt

I do not know how to apply suggested fixes for an easier diff.
I could run clang-tidy with some inappropriate check, so that it will fix all the Wdocumentation warnings only. But that takes forever.

Anyway based on the approach above I found two additional regressions. Testing with real code seems to be kind of important ;-)

• Reported column numbers have been changed. Apparently this cannot be checked by expected-warning and expected-note, so I added another FileCheck pass with a few select test cases.
• All warnings are reported first and then all the notes. Apparently this is not checked by expected-warning and expected-note. This will probably also require to be checked by FileCheck directly.

I'll further analyze the diff ASAP (have to fix the second problem first so that the diff gets smaller).
But that experience was kind of enough to get the following idea:
I would like to create two different patches:

• One with all the new tests (modified to pass based on current code). It should be trivial to review.
• One with the new code and the required small modifications to tests.

Because at the moment lots of new test cases are added and it's not clear what is even different now with this patch.
That seems to be the best way to ensure all changes are clearly visible within test cases.

What do you think of that @Mordante?

[Mordante]

I do not know how to apply suggested fixes for an easier diff.
I could run clang-tidy with some inappropriate check, so that it will fix all the Wdocumentation warnings only. But that takes forever.

Would it be feasible to run it on a part of the clang codebase just to get a feeling, or does that also take too long?

Anyway based on the approach above I found two additional regressions. Testing with real code seems to be kind of important ;-)

• Reported column numbers have been changed. Apparently this cannot be checked by expected-warning and expected-note, so I added another FileCheck pass with a few select test cases.

Maybe first look why the columns have changed and whether it's a real issue.

• All warnings are reported first and then all the notes. Apparently this is not checked by expected-warning and expected-note. This will probably also require to be checked by FileCheck directly.

AFAIK other diagnostic tests don't test the in which the diagnostics are emitted. So I wonder whether this is really required.

I'll further analyze the diff ASAP (have to fix the second problem first so that the diff gets smaller).
But that experience was kind of enough to get the following idea:
I would like to create two different patches:

• One with all the new tests (modified to pass based on current code). It should be trivial to review.
• One with the new code and the required small modifications to tests.

I think if would be first good to figure out why this changes and then see whether it's a real issue. If it's an issue it needs to be fixed, else the separate patches would be preferred.

clang/lib/AST/CommentSema.cpp

22

Yes please. When needed again for debugging it's trivial to add the wanted debug statements.

873

Yes if it's not trivial a separate patch would be preferable.

clang/test/Sema/warn-documentation-fixits.cpp

81

Yes maybe if the edit distance is the same it would be a good idea to use the case insensitive match. But I agree this can be done in a followup patch.

149

Would it be easy to only show the note on the first pass without the fixit? Then the user still gets a hint, but not the fixit.

[AlexanderLanin]

Anyway based on the approach above I found two additional regressions. Testing with real code seems to be kind of important ;-)

• Reported column numbers have been changed. Apparently this cannot be checked by expected-warning and expected-note, so I added another FileCheck pass with a few select test cases.

Maybe first look why the columns have changed and whether it's a real issue.

• All warnings are reported first and then all the notes. Apparently this is not checked by expected-warning and expected-note. This will probably also require to be checked by FileCheck directly.

AFAIK other diagnostic tests don't test the in which the diagnostics are emitted. So I wonder whether this is really required.

I think if would be first good to figure out why this changes and then see whether it's a real issue. If it's an issue it needs to be fixed, else the separate patches would be preferred.

I think those unintentional changes are an issue. See where ^ points and how the readability of the messages decreases due to their order:

TimeFormatTypes.h:171:14: warning: parameter 'paramGpsWeek:' not found in the function declaration [-Wdocumentation]
        /// @param paramGpsWeek: weeks till 6.1.1980
             ^     ~~~~~~~~~~~~~
TimeFormatTypes.h:172:14: warning: parameter 'paramGpsTimeOfWeek:' not found in the function declaration [-Wdocumentation]
        /// @param paramGpsTimeOfWeek: seconds in current week
             ^     ~~~~~~~~~~~~~~~~~~~
TimeFormatTypes.h:173:14: warning: parameter 'paramLeapSeconds:' not found in the function declaration [-Wdocumentation]
        /// @param paramLeapSeconds: leap seconds, default 0
             ^     ~~~~~~~~~~~~~~~~~
TimeFormatTypes.h:171:14: note: did you mean 'paramGpsWeek'?
        /// @param paramGpsWeek: weeks till 6.1.1980
             ^     ~~~~~~~~~~~~~
                   paramGpsWeek
TimeFormatTypes.h:172:14: note: did you mean 'paramGpsTimeOfWeek'?
        /// @param paramGpsTimeOfWeek: seconds in current week
             ^     ~~~~~~~~~~~~~~~~~~~
                   paramGpsTimeOfWeek
TimeFormatTypes.h:173:14: note: did you mean 'paramLeapSeconds'?
        /// @param paramLeapSeconds: leap seconds, default 0
             ^     ~~~~~~~~~~~~~~~~~
                   paramLeapSeconds

I'll fix this shortly.

clang/test/Sema/warn-documentation-fixits.cpp

91

Because at the time this is executed there are technically two parameters unmatched. One has a fix-it suggestion.

149

Love the idea, will look into it!

clang/include/clang/AST/CommentSema.h

public:
   void inspectThisDecl();
 
   /// Returns index of a function parameter with a given name.
   unsigned resolveParmVarReference(StringRef Name,
                                    ArrayRef<const ParmVarDecl *> ParamVars);
 
   /// Returns index of a function parameter with the name closest to a given
   /// typo.
-  unsigned correctTypoInParmVarReference(StringRef Typo,
-                                         ArrayRef<const ParmVarDecl *> ParamVars);
+  static unsigned
+  correctTypoInParmVarReference(StringRef Typo,
 
   bool resolveTParamReference(StringRef Name,
                               const TemplateParameterList *TemplateParameters,
                               SmallVectorImpl<unsigned> *Position);
 
   StringRef correctTypoInTParamReference(
                               StringRef Typo,
                               const TemplateParameterList *TemplateParameters);

clang/lib/AST/CommentSema.cpp

 #include "clang/AST/CommentDiagnostic.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/Debug.h"
+
+#define DEBUG_TYPE "commentsema"
 
 namespace clang {
 namespace comments {
 
 namespace {
 #include "clang/AST/CommentHTMLTagsProperties.inc"
 } // end anonymous namespace
 
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
     SmallString<64> TextToInsert = AttributeSpelling;
     TextToInsert += " ";
     SourceLocation Loc = FD->getSourceRange().getBegin();
     Diag(Loc, diag::note_add_deprecation_attr)
         << FixItHint::CreateInsertion(Loc, TextToInsert);
   }
 }
 
-void Sema::resolveParamCommandIndexes(const FullComment *FC) {
-  if (!isFunctionDecl()) {
-    // We already warned that \\param commands are not attached to a function
-    // decl.
-    return;
-  }
+/// Finds ParmVarDecl with matching Name
+/// @param IsFunctionOrMethodVariadic If true this will match Name "..." to
+/// VarArgParamIndex
+/// @return index of match within ParamVars, InvalidParamIndex if no match
+static unsigned resolveParmVarReference(StringRef Name,
+                                        ArrayRef<const ParmVarDecl *> ParamVars,
+                                        bool IsFunctionOrMethodVariadic) {
+  for (unsigned I = 0, E = ParamVars.size(); I != E; ++I) {
+    const IdentifierInfo *II = ParamVars[I]->getIdentifier();
+    if (II && II->getName() == Name)
+      return I;
+  }
+  if (Name == "..." && IsFunctionOrMethodVariadic)
+    return ParamCommandComment::VarArgParamIndex;
+  return ParamCommandComment::InvalidParamIndex;
 
-  SmallVector<ParamCommandComment *, 8> UnresolvedParamCommands;
-
-  // Comment AST nodes that correspond to \c ParamVars for which we have
-  // found a \\param command or NULL if no documentation was found so far.
-  SmallVector<ParamCommandComment *, 8> ParamVarDocs;
-
-  ArrayRef<const ParmVarDecl *> ParamVars = getParamVars();
-  ParamVarDocs.resize(ParamVars.size(), nullptr);
-
-  // First pass over all \\param commands: resolve all parameter names.
-  for (Comment::child_iterator I = FC->child_begin(), E = FC->child_end();
-       I != E; ++I) {
-    ParamCommandComment *PCC = dyn_cast<ParamCommandComment>(*I);
-    if (!PCC || !PCC->hasParamName())
-      continue;
+// Extracts all ParamCommandComments while setting IsVarArgParam and ParamIndex
+static SmallVector<const ParamCommandComment *, 8>
+extractParamCommandComments(const FullComment *const FC,
+                            ArrayRef<const ParmVarDecl *> ParamVars,
+                            const bool IsFunctionOrMethodVariadic) {
+  SmallVector<const ParamCommandComment *, 8> PCCs;
+
+  for (BlockContentComment *const Block : FC->getBlocks()) {
+    ParamCommandComment *const PCC = dyn_cast<ParamCommandComment>(Block);
+    if (PCC && PCC->hasParamName()) {
       StringRef ParamName = PCC->getParamNameAsWritten();
-
-    // Check that referenced parameter name is in the function decl.
-    const unsigned ResolvedParamIndex = resolveParmVarReference(ParamName,
-                                                                ParamVars);
-    if (ResolvedParamIndex == ParamCommandComment::VarArgParamIndex) {
+      const unsigned Index = resolveParmVarReference(
+          ParamName, ParamVars, IsFunctionOrMethodVariadic);
+      if (Index == ParamCommandComment::VarArgParamIndex)
         PCC->setIsVarArgParam();
-      continue;
-    }
-    if (ResolvedParamIndex == ParamCommandComment::InvalidParamIndex) {
-      UnresolvedParamCommands.push_back(PCC);
-      continue;
-    }
-    PCC->setParamIndex(ResolvedParamIndex);
-    if (ParamVarDocs[ResolvedParamIndex]) {
-      SourceRange ArgRange = PCC->getParamNameRange();
-      Diag(ArgRange.getBegin(), diag::warn_doc_param_duplicate)
-        << ParamName << ArgRange;
-      ParamCommandComment *PrevCommand = ParamVarDocs[ResolvedParamIndex];
-      Diag(PrevCommand->getLocation(), diag::note_doc_param_previous)
-        << PrevCommand->getParamNameRange();
+      else if (Index != ParamCommandComment::InvalidParamIndex)
+        PCC->setParamIndex(Index);
+
+      PCCs.push_back(PCC);
     }
-    ParamVarDocs[ResolvedParamIndex] = PCC;
   }
 
-  // Find parameter declarations that have no corresponding \\param.
-  SmallVector<const ParmVarDecl *, 8> OrphanedParamDecls;
-  for (unsigned i = 0, e = ParamVarDocs.size(); i != e; ++i) {
-    if (!ParamVarDocs[i])
-      OrphanedParamDecls.push_back(ParamVars[i]);
-  }
+  return PCCs;
 
-  // Second pass over unresolved \\param commands: do typo correction.
-  // Suggest corrections from a set of parameter declarations that have no
-  // corresponding \\param.
-  for (unsigned i = 0, e = UnresolvedParamCommands.size(); i != e; ++i) {
-    const ParamCommandComment *PCC = UnresolvedParamCommands[i];
-
-    SourceRange ArgRange = PCC->getParamNameRange();
-    StringRef ParamName = PCC->getParamNameAsWritten();
-    Diag(ArgRange.getBegin(), diag::warn_doc_param_not_found)
-      << ParamName << ArgRange;
-
-    // All parameters documented -- can't suggest a correction.
-    if (OrphanedParamDecls.size() == 0)
-      continue;
+/// Returns all ParmVarDecl without a comment. Diagnosis is emitted in case of
+/// mapping issues (no matching ParmVar or duplicate Comment).
+static SmallVector<const ParmVarDecl *, 8> mapParamCommentsToParmVars(
+    const SmallVector<const ParamCommandComment *, 8> &PCCs,
+    const ArrayRef<const ParmVarDecl *> &ParmVars, DiagnosticsEngine &Diags) {
+  // Matching ParamCommandComment by index of ParmVars.
+  // It's basically a map<ParmVarDecl, ParamCommandComment> in terms of
+  // readability, but vector is faster, as it usually contains only very few
+  // elements and the index is known on access. Downside is no support for
+  // VarArg.
+  SmallVector<const ParamCommandComment *, 8> ParmVarDeclToParamCommandComment(
+      ParmVars.size(), nullptr);
+
+  for (const ParamCommandComment *const PCC : PCCs) {
+    if (PCC->isVarArgParam()) {
+      // No additional diagnostic supported yet.
+    } else if (PCC->isParamIndexValid()) {
+      assert(PCC->getParamIndex() < ParmVars.size() &&
+             "PCCs and ParmVars vectors do not match");
+      // If there is a previous comment for the same ParmVarDecl, this is a
+      // duplicate.
+      if (const ParamCommandComment *const PrevPCC =
+              ParmVarDeclToParamCommandComment[PCC->getParamIndex()]) {
+        Diags.Report(PCC->getLocation(), diag::warn_doc_param_duplicate)
+            << PCC->getParamNameAsWritten() << PCC->getParamNameRange();
+        Diags.Report(PrevPCC->getLocation(), diag::note_doc_param_previous)
+            << PrevPCC->getParamNameRange();
+      }
+      // Store mapping of ParmVarDecl[index] to ParamCommandComment to detect
+      // duplicates (above) and to detect unmapped parameters (below).
+      // Overwrite in case of duplicate, it does not matter.
+      ParmVarDeclToParamCommandComment[PCC->getParamIndex()] = PCC;
+    } else
+      Diags.Report(PCC->getLocation(), diag::warn_doc_param_not_found)
+          << PCC->getParamNameAsWritten() << PCC->getParamNameRange();
+  }
+
+  SmallVector<const ParmVarDecl *, 8> ParmVarsWithoutComment;
+
+  for (unsigned I = 0, E = ParmVars.size(); I != E; ++I)
+    if (!ParmVarDeclToParamCommandComment[I])
+      ParmVarsWithoutComment.push_back(ParmVars[I]);
+
+  return ParmVarsWithoutComment;
+}
+
+static SmallVector<const ParamCommandComment *, 8>
+extractUnresolvedParamComments(
+    const SmallVector<const ParamCommandComment *, 8> &ParamComments) {
+  SmallVector<const ParamCommandComment *, 8> UnresolvedParamComments;
+
+  for (const ParamCommandComment *const PCC : ParamComments)
+    if (!PCC->isParamIndexValid())
+      UnresolvedParamComments.push_back(PCC);
+
+  return UnresolvedParamComments;
+}
+
+void attemptToMatchUnresolvedComments(
+    const SmallVector<const ParamCommandComment *, 8> &UnresolvedParamComments,
+    const SmallVector<const ParmVarDecl *, 8> &OrphanedParmVars,
+    DiagnosticsEngine &Diags) {
+  // Quick exit for the normal case where UnresolvedParamComments is empty.
+  if (UnresolvedParamComments.empty() || OrphanedParmVars.empty())
+    return;
+
+  // Corrected OrphanedParmVar by index of UnresolvedParamComments.
+  SmallVector<const ParmVarDecl *, 8> ParmVarForComment(
+      UnresolvedParamComments.size(), nullptr);
+
+  // ParmVarForComment index by index of OrphanedParmVars. This is for detecting
+  // the case where different comments are corrected to the same parameter.
+  // Note: Entries are never deleted and are therefore partially part of
+  // duplications.
+  SmallVector<unsigned, 8> CommentToParmVar(
+      OrphanedParmVars.size(), ParamCommandComment::InvalidParamIndex);
 
-    unsigned CorrectedParamIndex = ParamCommandComment::InvalidParamIndex;
-    if (OrphanedParamDecls.size() == 1) {
-      // If one parameter is not documented then that parameter is the only
-      // possible suggestion.
-      CorrectedParamIndex = 0;
+  if (UnresolvedParamComments.size() == 1 && OrphanedParmVars.size() == 1)
+    ParmVarForComment[0] = OrphanedParmVars.front();
+  else {
+    for (unsigned I = 0, E = UnresolvedParamComments.size(); I != E; ++I) {
+
+      StringRef ParamName = UnresolvedParamComments[I]->getParamNameAsWritten();
+
+      unsigned CorrectedParmVarIndex =
+          Sema::correctTypoInParmVarReference(ParamName, OrphanedParmVars);
+
+      if (CorrectedParmVarIndex != ParamCommandComment::InvalidParamIndex) {
+        // Correction suggestion exists (close match by name).
+
+        if (CommentToParmVar[CorrectedParmVarIndex] !=
+            ParamCommandComment::InvalidParamIndex) {
+          // There was already a suggestion to match a different comment to this
+          // parameter! Remove old and new correction.
+          LLVM_DEBUG(llvm::dbgs() << "Duplicate suggestion to correct @"
+                                  << ParamName << " to "
+                                  << OrphanedParmVars[CorrectedParmVarIndex]
+                                         ->getIdentifier()
+                                         ->getName()
+                                  << "\n");
+          ParmVarForComment[CommentToParmVar[CorrectedParmVarIndex]] = nullptr;
+          ParmVarForComment[I] = nullptr;
         } else {
-      // Do typo correction.
-      CorrectedParamIndex = correctTypoInParmVarReference(ParamName,
-                                                          OrphanedParamDecls);
-    }
-    if (CorrectedParamIndex != ParamCommandComment::InvalidParamIndex) {
-      const ParmVarDecl *CorrectedPVD = OrphanedParamDecls[CorrectedParamIndex];
-      if (const IdentifierInfo *CorrectedII = CorrectedPVD->getIdentifier())
-        Diag(ArgRange.getBegin(), diag::note_doc_param_name_suggestion)
+          // Store suggestion into vector for duplicate detection and into
+          // vector of suggested corrections.
+          LLVM_DEBUG(llvm::dbgs()
+                     << "Suggestion to correct @" << ParamName << " to "
+                     << OrphanedParmVars[CorrectedParmVarIndex]
+                            ->getIdentifier()
+                            ->getName()
+                     << "\n");
+          CommentToParmVar[CorrectedParmVarIndex] = I;
+          ParmVarForComment[I] = OrphanedParmVars[CorrectedParmVarIndex];
+        }
+      }
+    }
+  }
+
+  for (unsigned I = 0, E = UnresolvedParamComments.size(); I != E; ++I) {
+    if (ParmVarForComment[I]) {
+      if (const IdentifierInfo *CorrectedII =
+              ParmVarForComment[I]->getIdentifier()) {
+        const ParamCommandComment *PCC = UnresolvedParamComments[I];
+
+        Diags.Report(PCC->getLocation(), diag::note_doc_param_name_suggestion)
             << CorrectedII->getName()
-          << FixItHint::CreateReplacement(ArgRange, CorrectedII->getName());
-    }
-  }
+            << FixItHint::CreateReplacement(PCC->getParamNameRange(),
+                                            CorrectedII->getName());
+      }
+    }
+
+void Sema::resolveParamCommandIndexes(const FullComment *FC) {
+  if (!isFunctionDecl()) {
+    // We already warned that \\param commands are not attached to a function
+    // decl.
+    return;
+  }
+
+  const ArrayRef<const ParmVarDecl *> ParmVars = getParamVars();
+  const SmallVector<const ParamCommandComment *, 8> ParamComments =
+      extractParamCommandComments(FC, ParmVars, isFunctionOrMethodVariadic());
+
+  const SmallVector<const ParmVarDecl *, 8> ParmVarsWithoutComment =
+      mapParamCommentsToParmVars(ParamComments, ParmVars, Diags);
+
+  const SmallVector<const ParamCommandComment *, 8> UnresolvedParamComments =
+      extractUnresolvedParamComments(ParamComments);
+
+  attemptToMatchUnresolvedComments(UnresolvedParamComments,
+                                   ParmVarsWithoutComment, Diags);
 }
 
 bool Sema::isFunctionDecl() {
   if (!ThisDeclInfo)
     return false;
   if (!ThisDeclInfo->IsFilled)
     inspectThisDecl();
   return ThisDeclInfo->getKind() == DeclInfo::FunctionKind;
bool Sema::isUnionDecl() {
   if (!ThisDeclInfo->IsFilled)
     inspectThisDecl();
   if (const RecordDecl *RD =
         dyn_cast_or_null<RecordDecl>(ThisDeclInfo->CurrentDecl))
     return RD->isUnion();
   return false;
 }
 static bool isClassOrStructDeclImpl(const Decl *D) {
-  if (auto *record = dyn_cast_or_null<RecordDecl>(D))
-    return !record->isUnion();
+  if (const auto *Record = dyn_cast_or_null<RecordDecl>(D))
+    return !Record->isUnion();
 
   return false;
 }
 
 bool Sema::isClassOrStructDecl() {
   if (!ThisDeclInfo)
     return false;
   if (!ThisDeclInfo->IsFilled)
if (!ThisDeclInfo->IsFilled)
     inspectThisDecl();
 
   if (!ThisDeclInfo->CurrentDecl)
     return false;
 
   if (isClassOrStructDeclImpl(ThisDeclInfo->CurrentDecl))
     return true;
 
-  if (auto *ThisTypedefDecl = dyn_cast<TypedefDecl>(ThisDeclInfo->CurrentDecl)) {
+  if (const auto *ThisTypedefDecl =
+          dyn_cast<TypedefDecl>(ThisDeclInfo->CurrentDecl)) {
     auto UnderlyingType = ThisTypedefDecl->getUnderlyingType();
-    if (auto ThisElaboratedType = dyn_cast<ElaboratedType>(UnderlyingType)) {
+    if (const auto *ThisElaboratedType =
+            dyn_cast<ElaboratedType>(UnderlyingType)) {
       auto DesugaredType = ThisElaboratedType->desugar();
-      if (auto *DesugaredTypePtr = DesugaredType.getTypePtrOrNull()) {
-        if (auto *ThisRecordType = dyn_cast<RecordType>(DesugaredTypePtr)) {
+      if (const auto *DesugaredTypePtr = DesugaredType.getTypePtrOrNull()) {
+        if (const auto *ThisRecordType =
+                dyn_cast<RecordType>(DesugaredTypePtr)) {
           return isClassOrStructDeclImpl(ThisRecordType->getAsRecordDecl());
         }
       }
     }
   }
 
   return false;
 }

 }
 
 void Sema::inspectThisDecl() {
   ThisDeclInfo->fill();
 }
 
 unsigned Sema::resolveParmVarReference(StringRef Name,
                                        ArrayRef<const ParmVarDecl *> ParamVars) {
-  for (unsigned i = 0, e = ParamVars.size(); i != e; ++i) {
-    const IdentifierInfo *II = ParamVars[i]->getIdentifier();
-    if (II && II->getName() == Name)
-      return i;
-  }
-  if (Name == "..." && isFunctionOrMethodVariadic())
-    return ParamCommandComment::VarArgParamIndex;
-  return ParamCommandComment::InvalidParamIndex;
+  return ::clang::comments::resolveParmVarReference(
+      Name, ParamVars, isFunctionOrMethodVariadic());
 }
 
 namespace {
 class SimpleTypoCorrector {
   const NamedDecl *BestDecl;
 
   StringRef Typo;
   const unsigned MaxEditDistance;
public:
 
   unsigned getBestDeclIndex() const {
     assert(getBestDecl());
     return BestIndex;
   }
 };
 
 void SimpleTypoCorrector::addDecl(const NamedDecl *ND) {
-  unsigned CurrIndex = NextIndex++;
+  const unsigned CurrIndex = NextIndex++;
 
-  const IdentifierInfo *II = ND->getIdentifier();
+  const IdentifierInfo *const II = ND->getIdentifier();
   if (!II)
     return;
 
   StringRef Name = II->getName();
-  unsigned MinPossibleEditDistance = abs((int)Name.size() - (int)Typo.size());
-  if (MinPossibleEditDistance > 0 &&
-      Typo.size() / MinPossibleEditDistance < 3)
-    return;
-
-  unsigned EditDistance = Typo.edit_distance(Name, true, MaxEditDistance);
+  const unsigned EditDistance = Typo.edit_distance(Name, true, MaxEditDistance);
   if (EditDistance < BestEditDistance) {
     BestEditDistance = EditDistance;
     BestDecl = ND;
     BestIndex = CurrIndex;
   }
 }
 } // end anonymous namespace
 
 unsigned Sema::correctTypoInParmVarReference(
                                     StringRef Typo,
                                     ArrayRef<const ParmVarDecl *> ParamVars) {
   SimpleTypoCorrector Corrector(Typo);
-  for (unsigned i = 0, e = ParamVars.size(); i != e; ++i)
-    Corrector.addDecl(ParamVars[i]);
+  for (unsigned I = 0, E = ParamVars.size(); I != E; ++I)
+    Corrector.addDecl(ParamVars[I]);
   if (Corrector.getBestDecl())
     return Corrector.getBestDeclIndex();
   else
     return ParamCommandComment::InvalidParamIndex;
 }
 
 namespace {
-bool ResolveTParamReferenceHelper(
-                            StringRef Name,
-                            const TemplateParameterList *TemplateParameters,
-                            SmallVectorImpl<unsigned> *Position) {
-  for (unsigned i = 0, e = TemplateParameters->size(); i != e; ++i) {
-    const NamedDecl *Param = TemplateParameters->getParam(i);
+bool resolveTParamReferenceHelper(
+    StringRef Name, const TemplateParameterList *TemplateParameters,
+  for (unsigned I = 0, E = TemplateParameters->size(); I != E; ++I) {
+    const NamedDecl *Param = TemplateParameters->getParam(I);
     const IdentifierInfo *II = Param->getIdentifier();
     if (II && II->getName() == Name) {
-      Position->push_back(i);
+      Position->push_back(I);
       return true;
     }
 
     if (const TemplateTemplateParmDecl *TTP =
             dyn_cast<TemplateTemplateParmDecl>(Param)) {
-      Position->push_back(i);
-      if (ResolveTParamReferenceHelper(Name, TTP->getTemplateParameters(),
+      Position->push_back(I);
+      if (resolveTParamReferenceHelper(Name, TTP->getTemplateParameters(),
                                        Position))
         return true;
       Position->pop_back();
     }
   }
   return false;
 }
 } // end anonymous namespace
 
 bool Sema::resolveTParamReference(
                             StringRef Name,
                             const TemplateParameterList *TemplateParameters,
                             SmallVectorImpl<unsigned> *Position) {
   Position->clear();
   if (!TemplateParameters)
     return false;
 
-  return ResolveTParamReferenceHelper(Name, TemplateParameters, Position);
+  return resolveTParamReferenceHelper(Name, TemplateParameters, Position);
 }
 
 namespace {
-void CorrectTypoInTParamReferenceHelper(
-                            const TemplateParameterList *TemplateParameters,
-                            SimpleTypoCorrector &Corrector) {
-  for (unsigned i = 0, e = TemplateParameters->size(); i != e; ++i) {
-    const NamedDecl *Param = TemplateParameters->getParam(i);
+void correctTypoInTParamReferenceHelper(
+  for (unsigned I = 0, E = TemplateParameters->size(); I != E; ++I) {
+    const NamedDecl *Param = TemplateParameters->getParam(I);
     Corrector.addDecl(Param);
 
     if (const TemplateTemplateParmDecl *TTP =
             dyn_cast<TemplateTemplateParmDecl>(Param))
-      CorrectTypoInTParamReferenceHelper(TTP->getTemplateParameters(),
+      correctTypoInTParamReferenceHelper(TTP->getTemplateParameters(),
                                          Corrector);
   }
 }
 } // end anonymous namespace
 
 StringRef Sema::correctTypoInTParamReference(
                             StringRef Typo,
                             const TemplateParameterList *TemplateParameters) {
   SimpleTypoCorrector Corrector(Typo);
-  CorrectTypoInTParamReferenceHelper(TemplateParameters, Corrector);
+  correctTypoInTParamReferenceHelper(TemplateParameters, Corrector);
   if (const NamedDecl *ND = Corrector.getBestDecl()) {
     const IdentifierInfo *II = ND->getIdentifier();
     assert(II && "SimpleTypoCorrector should not return this decl");
     return II->getName();
   }
   return StringRef();
 }
 

clang/test/Sema/warn-documentation-fixits.cpp

 // RUN: %clang_cc1 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -verify %s
 // RUN  %clang_cc1 -std=c++11 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck -DATTRIBUTE="__attribute__((deprecated))" %s
 // RUN: %clang_cc1 -std=c++14 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK14 -DATTRIBUTE="[[deprecated]]" %s
 
-// expected-warning@+1 {{parameter 'ZZZZZZZZZZ' not found in the function declaration}} expected-note@+1 {{did you mean 'a'?}}
+// expected-warning@+2 {{parameter 'ZZZZZZZZZZ' not found in the function declaration}} expected-note@+2 {{did you mean 'a'?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:12-[[@LINE+1]]:22}:"a"
 /// \param ZZZZZZZZZZ Blah blah.
-int test1(int a);
+int fixSinceThereIsOnlyOneOption(int a);
 
-// expected-warning@+1 {{parameter 'aab' not found in the function declaration}} expected-note@+1 {{did you mean 'aaa'?}}
+// expected-warning@+2 {{parameter 'aab' not found in the function declaration}} expected-note@+2 {{did you mean 'aaa'?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:12-[[@LINE+1]]:15}:"aaa"
 /// \param aab Blah blah.
-int test2(int aaa, int bbb);
+int fixParamCommentToBestMatch(int aaa, int bbb);
 
-// expected-warning@+1 {{template parameter 'ZZZZZZZZZZ' not found in the template declaration}} expected-note@+1 {{did you mean 'T'?}}
+// expected-warning@+3 {{parameter 'aab' not found in the function declaration}} expected-note-NOT@+3 {{did you mean}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+2]]:
+/// \param aaa Documented
+/// \param aab Do not duplicate documentation of aaa.
+int doNotDuplicateExistingComments(int aaa);
+
+// expected-warning@+4 {{parameter 'aaaa' not found in the function declaration}} expected-note@+4 {{did you mean 'aaa'?}}
+// expected-warning@+4 {{parameter 'ThisOneHasBeenRemovedButIsStillDocumented' not found in the function declaration}} expected-note-NOT@+4 {{did you mean}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+2]]:
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+2]]:
+/// \param aaaa Almost there.
+/// \param ThisOneHasBeenRemovedButIsStillDocumented Widely different name!
+int doNotDuplicateCommentsViaFixIt(int aaa);
+
+// Test data and expectations from https://bugs.llvm.org/show_bug.cgi?id=43755.
+// Note: there is a an intentional typo in sligtlyMisspelled.
+// expected-warning@+4 {{parameter 'sligtlyMisspelled' not found in the function declaration}} expected-note@+4 {{did you mean 'slightlyMisspelled'?}}
+// expected-warning@+4 {{parameter 'someParameterWhichWasRemovedLongAgo' not found in the function declaration}} expected-note-NOT@+4 {{did you mean}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+2]]:
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+2]]:
+/// @param sligtlyMisspelled hello
+/// @param someParameterWhichWasRemovedLongAgo world
+void doNotDuplicateCommentsViaFixIt_43755(int slightlyMisspelled);
+
+// expected-warning@+6 {{parameter 'aaaa' not found in the function declaration}} expected-note-NOT@+6 {{did you mean}}
+// expected-warning@+6 {{parameter 'aaab' not found in the function declaration}} expected-note-NOT@+6 {{did you mean}}
+// expected-warning@+6 {{parameter 'aaac' not found in the function declaration}} expected-note-NOT@+6 {{did you mean}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+3]]:
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+3]]:
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+3]]:
+/// \param aaaa Almost there!
+/// \param aaab Me too!
+/// \param aaac Me three!
+int onlyFixWhenExactlyOneBestParamComment(int aaa);
+
+// expected-warning@+6 {{parameter 'aaab' not found in the function declaration}} expected-note-NOT@+6 {{did you mean}}
+// expected-warning@+6 {{parameter 'aabb' not found in the function declaration}} expected-note-NOT@+6 {{did you mean}}
+// expected-warning@+6 {{parameter 'abbb' not found in the function declaration}} expected-note-NOT@+6 {{did you mean}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+3]]:
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+3]]:
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+3]]:
+/// \param aaab Almost there!
+/// \param aabb Me too!
+/// \param abbb Me three!
+int onlyFixWhenExactlyOneBestParamComment_WithUnrelatedParam(int aaa, int NotMe);
+
+// expected-warning@+6 {{parameter 'aaab' not found in the function declaration}} expected-note-NOT@+6 {{did you mean}}
+// expected-warning@+6 {{parameter 'aabb' not found in the function declaration}} expected-note-NOT@+6 {{did you mean}}
+// expected-warning@+6 {{parameter 'abbb' not found in the function declaration}} expected-note-NOT@+6 {{did you mean}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+3]]:
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+3]]:
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+3]]:
+/// \param aaab Almost there!
+/// \param aabb Me too!
+/// \param abbb Me three!
+/// \param NotMe Match!
+int onlyFixWhenExactlyOneBestParamComment_WithUnrelatedParam2(int aaa, int NotMe);
+
+// Auto-fixing those two parameters would potentially worsen the situation as information is lost.
+// At the moment they at least have different names and a human can resonably pick the correct one by name.
+// expected-warning@+4 {{parameter 'ArgC' not found in the function declaration}} expected-note-NOT@+4 {{did you mean}}
+// expected-warning@+4 {{parameter 'ArgV' not found in the function declaration}} expected-note-NOT@+4 {{did you mean}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+2]]:
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+2]]:
+/// \param ArgC Almost matches.
+/// \param ArgV Almost matches.
+int test2to2_bothAlmost(int argc, int argv);
+
+// Note: first parameter is the same as above, bug gets auto fixed here since
+// it's obviously the better choice.
+// expected-warning@+4 {{parameter 'ArgC' not found in the function declaration}} expected-note@+4 {{did you mean}}
+// expected-warning@+4 {{parameter 'ZZZZZZ' not found in the function declaration}} expected-note-NOT@+4 {{did you mean}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+2]]:
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+2]]:
+/// \param ArgC Almost matches.
+/// \param ZZZZZZ Does not match at all.
+int test2to2_oneAlmost(int argc, int argv);
+
+// Note: first parameter is the same as above, bug gets auto fixed here since
+// the other is already ok.
+// expected-warning@+4 {{parameter 'ArgC' not found in the function declaration}} expected-note@+4 {{did you mean}}
+// expected-warning-NOT@+4 {{parameter 'ZZZZZZ' not found in the function declaration}} expected-note-NOT@+4 {{did you mean}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+2]]:
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+2]]:
+/// \param ArgC Almost matches.
+/// \param argv Already matches.
+int test2to2_oneAlmost2(int argc, int argv);
+
+// expected-warning@+4 {{parameter 'YYYYYY' not found in the function declaration}} expected-note-NOT@+4 {{did you mean}}
+// expected-warning@+4 {{parameter 'ZZZZZZ' not found in the function declaration}} expected-note-NOT@+4 {{did you mean}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+2]]:
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+2]]:
+/// \param YYYYYY Does not match at all.
+/// \param ZZZZZZ Does not match at all.
+int test2to2_noMatch(int argc, int argv);
+
+// expected-warning@+7 {{parameter 'aaab' not found in the function declaration}} expected-note@+7 {{did you mean 'aab'?}}
+// expected-warning@+7 {{parameter 'aaac' not found in the function declaration}} expected-note@+7 {{did you mean 'aac'?}}
+// expected-warning@+7 {{parameter 'aaad' not found in the function declaration}} expected-note@+7 {{did you mean 'aad'?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+4]]:
+// CHECK: fix-it:"{{.*}}":{[[@LINE+4]]:
+// CHECK: fix-it:"{{.*}}":{[[@LINE+4]]:
+/**
+ * \param aaab Should match aab
+ * \param aaac Should match aac
+ * \param aaad Should match aad
+ */
+void test3to3_allAlmost(int aab, int aac, int aad);
+
+// expected-warning@+7 {{parameter 'aaab' not found in the function declaration}} expected-note@+7 {{did you mean 'aab'?}}
+// expected-warning@+7 {{parameter 'aaac' not found in the function declaration}} expected-note@+7 {{did you mean 'aac'?}}
+// expected-warning@+7 {{parameter 'aaad' not found in the function declaration}} expected-note@+7 {{did you mean 'aad'?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+4]]:
+// CHECK: fix-it:"{{.*}}":{[[@LINE+4]]:
+// CHECK: fix-it:"{{.*}}":{[[@LINE+4]]:
+/**
+ * \param aaab Should match aab
+ * \param aaac Should match aac
+ * \param aaad Should match aad
+ */
+void test3to3_allAlmost_rev(int aad, int aac, int aab);
+
+
+
+
+// Test data and expectations from https://bugs.llvm.org/show_bug.cgi?id=43808
+// expected-warning@+5 {{parameter 'arg[]' not found in the function declaration}} expected-note@+5 {{did you mean 'arg'?}}
+// expected-warning@+5 {{parameter 'cnt' not found in the function declaration}} expected-note-NOT@+5 {{did you mean 'argc'?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+3]]:{{.*}}:"arg"
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE+3]]:
+/**
+  *  @param[in] arg[]       Docu_1
+  *  @param[in] cnt         Docu_2
+  */
+void run(const char *const arg[], int argc);
+
+// 2nd run after the above has been fixed. Now 1 parameter needs fixing and
+// there is only one option, so it's matched regardless of how close they are.
+// (see also fixSinceThereIsOnlyOneOption test).
+// This could be handled in a single pass in the future.
+// expected-warning@+4 {{parameter 'cnt' not found in the function declaration}} expected-note@+4 {{did you mean 'argc'?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+3]]:{{.*}}:"argc"
+/**
+  *  @param[in] arg         Docu_1
+  *  @param[in] cnt         Docu_2
+  */
+void run(const char *const arg[], int argc);
+
+
+
+
+// expected-warning@+2 {{template parameter 'ZZZZZZZZZZ' not found in the template declaration}} expected-note@+2 {{did you mean 'T'?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:13-[[@LINE+1]]:23}:"T"
 /// \tparam ZZZZZZZZZZ Aaa
 template<typename T>
 void test3(T aaa);
 
-// expected-warning@+1 {{template parameter 'SomTy' not found in the template declaration}} expected-note@+1 {{did you mean 'SomeTy'?}}
+// expected-warning@+2 {{template parameter 'SomTy' not found in the template declaration}} expected-note@+2 {{did you mean 'SomeTy'?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:13-[[@LINE+1]]:18}:"SomeTy"
 /// \tparam SomTy Aaa
 /// \tparam OtherTy Bbb
 template<typename SomeTy, typename OtherTy>
 void test4(SomeTy aaa, OtherTy bbb);
 
-// expected-warning@+1 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+// expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+3 {{add a deprecation attribute to the declaration to silence this warning}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+2]]:1-[[@LINE+2]]:1}:"[[ATTRIBUTE]] "
 /// \deprecated
 void test_deprecated_1();
 
-// expected-warning@+1 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+// expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+3 {{add a deprecation attribute to the declaration to silence this warning}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+2]]:1-[[@LINE+2]]:1}:"[[ATTRIBUTE]] "
 /// \deprecated
 void test_deprecated_2(int a);
 
 struct test_deprecated_3 {
-  // expected-warning@+1 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+3 {{add a deprecation attribute to the declaration to silence this warning}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+2]]:3-[[@LINE+2]]:3}:"[[ATTRIBUTE]] "
   /// \deprecated
   void test_deprecated_4();
 
-  // expected-warning@+1 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+3 {{add a deprecation attribute to the declaration to silence this warning}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+2]]:3-[[@LINE+2]]:3}:"[[ATTRIBUTE]] "
   /// \deprecated
   void test_deprecated_5() {
   }
 };
 
 template<typename T>
 struct test_deprecated_6 {
-  // expected-warning@+1 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+3 {{add a deprecation attribute to the declaration to silence this warning}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+2]]:3-[[@LINE+2]]:3}:"[[ATTRIBUTE]] "
   /// \deprecated
   void test_deprecated_7();
 
-  // expected-warning@+1 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+3 {{add a deprecation attribute to the declaration to silence this warning}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+2]]:3-[[@LINE+2]]:3}:"[[ATTRIBUTE]] "
   /// \deprecated
   void test_deprecated_8() {
   }
 };
 
 class PR43753 {
-  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
-  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  // expected-warning@+3 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+3 {{add a deprecation attribute to the declaration to silence this warning}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+2]]:3-[[@LINE+2]]:3}:"[[ATTRIBUTE]] "
   /// \deprecated
   static void test_deprecated_static();
 
-  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
-  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  // expected-warning@+3 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+3 {{add a deprecation attribute to the declaration to silence this warning}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+2]]:3-[[@LINE+2]]:3}:"[[ATTRIBUTE]] "
   /// \deprecated
   static auto test_deprecated_static_trailing_return() -> int;
 
 #if __cplusplus >= 201402L
   // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
   // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
   /// \deprecated
   static decltype(auto) test_deprecated_static_decltype_auto() { return 1; }
 #endif
 
   // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
   // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
   /// \deprecated
   void test_deprecated_const() const;
 
   // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
   // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
   /// \deprecated
   auto test_deprecated_trailing_return() -> int;
 
 #if __cplusplus >= 201402L
-  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
-  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  // expected-warning@+3 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+3 {{add a deprecation attribute to the declaration to silence this warning}}
+  // CHECK14: fix-it:"{{.*}}":{[[@LINE+2]]:3-[[@LINE+2]]:3}:"[[ATTRIBUTE]] "
   /// \deprecated
   decltype(auto) test_deprecated_decltype_auto() const { return a; }
 
 private:
   int a{0};
 #endif
 };
 #define MY_ATTR_DEPRECATED __attribute__((deprecated))
 
-// expected-warning@+1 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+// expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+3 {{add a deprecation attribute to the declaration to silence this warning}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+2]]:1-[[@LINE+2]]:1}:"MY_ATTR_DEPRECATED "
 /// \deprecated
 void test_deprecated_9(int a);
 
 #if __cplusplus >= 201402L
 #define ATTRIBUTE_DEPRECATED [[deprecated]]
 
-// expected-warning@+1 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+// expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+3 {{add a deprecation attribute to the declaration to silence this warning}}
+// CHECK14: fix-it:"{{.*}}":{[[@LINE+2]]:1-[[@LINE+2]]:1}:"ATTRIBUTE_DEPRECATED "
 /// \deprecated
 void test_deprecated_10(int a);
 #endif
 
 // rdar://12381408
-// expected-warning@+2  {{unknown command tag name 'retur'; did you mean 'return'?}}
+// expected-warning@+3  {{unknown command tag name 'retur'; did you mean 'return'?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+2]]:6-[[@LINE+2]]:11}:"return"
 /// \brief testing fixit
 /// \retur int in FooBar
 int FooBar();
 
-// expected-warning@+1  {{unknown command tag name 'fooba'; did you mean 'foobar'?}}
+// expected-warning@+2  {{unknown command tag name 'fooba'; did you mean 'foobar'?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:6-[[@LINE+1]]:11}:"foobar"
 /// \fooba bbb IS_DOXYGEN_END
 int gorf();
 
 // expected-warning@+1 {{unknown command tag name}}
 /// \t bbb IS_DOXYGEN_END
 int Bar();
 
-// expected-warning@+2  {{unknown command tag name 'encode'; did you mean 'endcode'?}}
-// expected-warning@+1  {{'\endcode' command does not terminate a verbatim text block}}
+// expected-warning@+3  {{unknown command tag name 'encode'; did you mean 'endcode'?}}
+// expected-warning@+2  {{'\endcode' command does not terminate a verbatim text block}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:6-[[@LINE+1]]:12}:"endcode"
 /// \encode PR18051
 int PR18051();
 
-// CHECK: fix-it:"{{.*}}":{6:12-6:22}:"a"
-// CHECK: fix-it:"{{.*}}":{10:12-10:15}:"aaa"
-// CHECK: fix-it:"{{.*}}":{14:13-14:23}:"T"
-// CHECK: fix-it:"{{.*}}":{19:13-19:18}:"SomeTy"
-// CHECK: fix-it:"{{.*}}":{26:1-26:1}:"[[ATTRIBUTE]] "
-// CHECK: fix-it:"{{.*}}":{30:1-30:1}:"[[ATTRIBUTE]] "
-// CHECK: fix-it:"{{.*}}":{35:3-35:3}:"[[ATTRIBUTE]] "
-// CHECK: fix-it:"{{.*}}":{39:3-39:3}:"[[ATTRIBUTE]] "
-// CHECK: fix-it:"{{.*}}":{47:3-47:3}:"[[ATTRIBUTE]] "
-// CHECK: fix-it:"{{.*}}":{51:3-51:3}:"[[ATTRIBUTE]] "
-// CHECK: fix-it:"{{.*}}":{76:3-76:3}:"[[ATTRIBUTE]] "
-// CHECK: fix-it:"{{.*}}":{81:3-81:3}:"[[ATTRIBUTE]] "
-// CHECK14: fix-it:"{{.*}}":{87:3-87:3}:"[[ATTRIBUTE]] "
-// CHECK: fix-it:"{{.*}}":{97:1-97:1}:"MY_ATTR_DEPRECATED "
-// CHECK14: fix-it:"{{.*}}":{104:1-104:1}:"ATTRIBUTE_DEPRECATED "
-// CHECK: fix-it:"{{.*}}":{110:6-110:11}:"return"
-// CHECK: fix-it:"{{.*}}":{114:6-114:11}:"foobar"
-// CHECK: fix-it:"{{.*}}":{123:6-123:12}:"endcode"

clang/test/Sema/warn-documentation.cpp

class test_attach33 {
   /// \brief\author Aaa
   /// \tparam T Aaa
   template<typename T, typename U>
   void test_attach34(T aaa, U bbb);
 };
 
 template<typename T>
 class test_attach35 {
-  // expected-warning@+2 {{empty paragraph passed to '\brief' command}}
-  // expected-warning@+2 {{template parameter 'T' not found in the template declaration}}
+  // expected-warning@+3 {{empty paragraph passed to '\brief' command}}
+  // expected-warning@+3 {{template parameter 'T' not found in the template declaration}}
+  // expected-note@+2 {{did you mean 'TT'?}}
   /// \brief\author Aaa
   /// \tparam T Aaa
   template<typename TT, typename UU>
   void test_attach36(TT aaa, UU bbb);
 };
 
 // expected-warning@+2 {{empty paragraph passed to '\brief' command}}
 // expected-warning@+2 {{template parameter 'T' not found in the template declaration}}

llvm/unittests/ADT/StringRefTest.cpp

TEST(StringRefTest, Count) {
   StringRef NonOverlappingAbba("abbaabba");
   EXPECT_EQ(2U, NonOverlappingAbba.count("abba"));
   StringRef ComplexAbba("abbabbaxyzabbaxyz");
   EXPECT_EQ(2U, ComplexAbba.count("abba"));
 }
 
 TEST(StringRefTest, EditDistance) {
   StringRef Hello("hello");
+  EXPECT_EQ(2U, Hello.edit_distance("hellooo"));
+  EXPECT_EQ(2U, Hello.edit_distance("hel"));
+  EXPECT_EQ(2U, Hello.edit_distance("Hallo"));
   EXPECT_EQ(2U, Hello.edit_distance("hill"));
 
   StringRef Industry("industry");
   EXPECT_EQ(6U, Industry.edit_distance("interest"));
 
   StringRef Soylent("soylent green is people");
   EXPECT_EQ(19U, Soylent.edit_distance("people soiled our green"));
   EXPECT_EQ(26U, Soylent.edit_distance("people soiled our green",
                                       /* allow replacements = */ false));
   EXPECT_EQ(9U, Soylent.edit_distance("people soiled our green",
                                       /* allow replacements = */ true,