[analyzer] Extend bug reports with extra notes - CloneChecker

[NoQ]

Summary

This patch applies the bug reporting technique that is being introduced in D24278 to CloneChecker. Extra notes highlight cloned code sections.

Examples of HTML reports:

clone-checker-normal.html6 KBDownload

clone-checker-notes-as-events.html6 KBDownload

I slightly fixed the clone checker diagnostics. In particular, analyzer diagnostics, unlike clang diagnostics, cannot end with a period; and i also capitalized the first letter. Also made slight changes to the suspicious clone structure, because path pieces require more than source ranges to operate (didn't look why though).

Originally D24278 contained this patch, but then it was decided to make a separate review. The original diff is an exact copy of the relevant section of the original diff in D24278, with no review comments addressed.

[NoQ]

Addressed inline comments on the original review D24278. Rebased on top of the updated D24278. Changed warning messages completely according to Anna's comments in D24278.

[v.g.vassilev]

lib/StaticAnalyzer/Checkers/CloneChecker.cpp

102 ↗

(On Diff #72521)

I was thinking whether we shouldn't say code snippet or something that qualifies code, as it is too generic term.

112 ↗

(On Diff #72521)

Is that missing an indent?

[zaks.anna]

test/Analysis/copypaste/functions.cpp

7 ↗

(On Diff #72521)

"was" -> "is"?
Do we use past or present elsewhere?

test/Analysis/copypaste/suspicious-clones.cpp

61 ↗

(On Diff #72521)

The error message seems too verbose and focused on the implementation rather than user (ex: "suspicious code clone" and "suggestion is based").

Maybe we could say something like this:

• Did you mean to use 'a'?
• Similar code snippet here

[zaks.anna]

test/Analysis/copypaste/suspicious-clones.cpp

61 ↗

(On Diff #72521)

Better:

Did you mean to use 'a'?
Similar code snippet here uses 'b'

Did you mean to use 'a' instead of 'b'?
Similar code snippet here

[zaks.anna]

test/Analysis/copypaste/macros.cpp

8 ↗

(On Diff #72521)

• Duplicate code detected
• Similar code here

test/Analysis/copypaste/suspicious-clones.cpp

61 ↗

(On Diff #72521)

Another refinement

Potential copy-paste error: did you mean to use 'a' instead of 'b'?
Similar code here

[zaks.anna]

test/Analysis/copypaste/suspicious-clones.cpp

61 ↗

(On Diff #72521)

Potential copy-paste error; did you really mean to use 'b' here?
Similar code using 'a' here
Similar code using 'c' here

[NoQ]

• Update the warning messages.
• Move a run-away test file to its correct directory.
• Fix a copy-paste error in initialization of the variable field of a clone pair.

[zaks.anna]

LGTM!

[vitalybuka]

reverted with r283182

[vitalybuka]

reverted with r283182 as it depends on r283092

[chapuni]

cfe/trunk/lib/Analysis/CloneDetection.cpp

113

You should update \param here. See also r283106.

[vitalybuka]

Out of curiosity, how was MSVC crash solved?

[NoQ]

In D24916#561369, @vitalybuka wrote:

Out of curiosity, how was MSVC crash solved?

It wasn't, unfortunately; phabricator closed this revision by looking at the commit that was already reverted, with a delay (see http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20161003/172501.html for more info on the delay).

I'm planning to systematically split D24278 into smaller chunks, looking closely at the buildbot, in order to see what exactly breaks the MSVC compiler.
That would produce a lot of noise, for which i'm sorry in advance, but i don't seem to have better options(?)

[NoQ]

cfe/trunk/lib/Analysis/CloneDetection.cpp

113

Thanks!! Will do.

cfe/trunk/include/clang/Analysis/CloneDetection.h

public:
   /// This method should only be called on a non-empty StmtSequence object.
   SourceLocation getStartLoc() const;
 
   /// Returns the end sourcelocation of the last statement in this sequence.
   ///
   /// This method should only be called on a non-empty StmtSequence object.
   SourceLocation getEndLoc() const;
 
+  /// Returns the source range of the whole sequence - from the beginning
+  /// of the first statement to the end of the last statement.
+  SourceRange getSourceRange() const;
+
   bool operator==(const StmtSequence &Other) const {
     return std::tie(S, StartIndex, EndIndex) ==
            std::tie(Other.S, Other.StartIndex, Other.EndIndex);
   }
 
   bool operator!=(const StmtSequence &Other) const {
     return std::tie(S, StartIndex, EndIndex) !=
            std::tie(Other.S, Other.StartIndex, Other.EndIndex);
public:
   ///        pattern which could indicate a programming error.
   struct SuspiciousClonePair {
     /// \brief Utility class holding the relevant information about a single
     ///        clone in this pair.
     struct SuspiciousCloneInfo {
       /// The variable which referencing in this clone was against the pattern.
       const VarDecl *Variable;
       /// Where the variable was referenced.
-      SourceRange VarRange;
+      const Stmt *Mention;
       /// The variable that should have been referenced to follow the pattern.
       /// If Suggestion is a nullptr then it's not possible to fix the pattern
       /// by referencing a different variable in this clone.
       const VarDecl *Suggestion;
-      SuspiciousCloneInfo(const VarDecl *Variable, SourceRange Range,
+      SuspiciousCloneInfo(const VarDecl *Variable, const Stmt *Mention,
                           const VarDecl *Suggestion)
-          : Variable(Variable), VarRange(Range), Suggestion(Suggestion) {}
+          : Variable(Variable), Mention(Mention), Suggestion(Suggestion) {}
       SuspiciousCloneInfo() {}
     };
     /// The first clone in the pair which always has a suggested variable.
     SuspiciousCloneInfo FirstCloneInfo;
     /// This other clone in the pair which can have a suggested variable.
     SuspiciousCloneInfo SecondCloneInfo;
   };
 

cfe/trunk/lib/Analysis/CloneDetection.cpp

 }
 
 SourceLocation StmtSequence::getStartLoc() const {
   return front()->getLocStart();
 }
 
 SourceLocation StmtSequence::getEndLoc() const { return back()->getLocEnd(); }
 
+SourceRange StmtSequence::getSourceRange() const {
+  return SourceRange(getStartLoc(), getEndLoc());
+}
+
 namespace {
 
 /// \brief Analyzes the pattern of the referenced variables in a statement.
 class VariablePattern {
 
   /// \brief Describes an occurence of a variable reference in a statement.
   struct VariableOccurence {
     /// The index of the associated VarDecl in the Variables vector.
     size_t KindID;
-    /// The source range in the code where the variable was referenced.
-    SourceRange Range;
+    /// The statement in the code where the variable was referenced.
+    const Stmt *Mention;
 
-    VariableOccurence(size_t KindID, SourceRange Range)
-        : KindID(KindID), Range(Range) {}
+    VariableOccurence(size_t KindID, const Stmt *Mention)
+        : KindID(KindID), Mention(Mention) {}
   };
 
   /// All occurences of referenced variables in the order of appearance.
   std::vector<VariableOccurence> Occurences;
   /// List of referenced variables in the order of appearance.
   /// Every item in this list is unique.
   std::vector<const VarDecl *> Variables;
 
   /// \brief Adds a new variable referenced to this pattern.
   /// \param VarDecl The declaration of the variable that is referenced.
   /// \param Range The SourceRange where this variable is referenced.
-  void addVariableOccurence(const VarDecl *VarDecl, SourceRange Range) {
+  void addVariableOccurence(const VarDecl *VarDecl, const Stmt *Mention) {
     // First check if we already reference this variable
     for (size_t KindIndex = 0; KindIndex < Variables.size(); ++KindIndex) {
       if (Variables[KindIndex] == VarDecl) {
         // If yes, add a new occurence that points to the existing entry in
         // the Variables vector.
-        Occurences.emplace_back(KindIndex, Range);
+        Occurences.emplace_back(KindIndex, Mention);
         return;
       }
     }
     // If this variable wasn't already referenced, add it to the list of
     // referenced variables and add a occurence that points to this new entry.
-    Occurences.emplace_back(Variables.size(), Range);
+    Occurences.emplace_back(Variables.size(), Mention);
     Variables.push_back(VarDecl);
   }
 
   /// \brief Adds each referenced variable from the given statement.
   void addVariables(const Stmt *S) {
     // Sometimes we get a nullptr (such as from IfStmts which often have nullptr
     // children). We skip such statements as they don't reference any
     // variables.
     if (!S)
       return;
 
     // Check if S is a reference to a variable. If yes, add it to the pattern.
     if (auto D = dyn_cast<DeclRefExpr>(S)) {
       if (auto VD = dyn_cast<VarDecl>(D->getDecl()->getCanonicalDecl()))
-        addVariableOccurence(VD, D->getSourceRange());
+        addVariableOccurence(VD, D);
     }
 
     // Recursively check all children of the given statement.
     for (const Stmt *Child : S->children()) {
       addVariables(Child);
     }
   }
 
for (unsigned i = 0; i < Occurences.size(); ++i) {
       // which wouldn't break the pattern if it is used in place of the
       // current variable, we provide this variable as the suggested fix.
       if (OtherOccurence.KindID < Variables.size())
         FirstSuggestion = Variables[OtherOccurence.KindID];
 
       // Store information about the first clone.
       FirstMismatch->FirstCloneInfo =
           CloneDetector::SuspiciousClonePair::SuspiciousCloneInfo(
-              Variables[ThisOccurence.KindID], ThisOccurence.Range,
+              Variables[ThisOccurence.KindID], ThisOccurence.Mention,
               FirstSuggestion);
 
       // Same as above but with the other clone. We do this for both clones as
       // we don't know which clone is the one containing the unintended
       // pattern error.
       const VarDecl *SecondSuggestion = nullptr;
       if (ThisOccurence.KindID < Other.Variables.size())
         SecondSuggestion = Other.Variables[ThisOccurence.KindID];
 
       // Store information about the second clone.
       FirstMismatch->SecondCloneInfo =
           CloneDetector::SuspiciousClonePair::SuspiciousCloneInfo(
-              Variables[ThisOccurence.KindID], OtherOccurence.Range,
+              Other.Variables[OtherOccurence.KindID], OtherOccurence.Mention,
               SecondSuggestion);
 
       // SuspiciousClonePair guarantees that the first clone always has a
       // suggested variable associated with it. As we know that one of the two
       // clones in the pair always has suggestion, we swap the two clones
       // in case the first clone has no suggested variable which means that
       // the second clone has a suggested variable and should be first.
       if (!FirstMismatch->FirstCloneInfo.Suggestion)

cfe/trunk/lib/StaticAnalyzer/Checkers/CloneChecker.cpp

 /// CloneChecker is a checker that reports clones in the current translation
 /// unit.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "ClangSACheckers.h"
 #include "clang/Analysis/CloneDetection.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 
 using namespace clang;
 using namespace ento;
 
 namespace {
 class CloneChecker
     : public Checker<check::ASTCodeBody, check::EndOfTranslationUnit> {
   mutable CloneDetector Detector;
+  mutable std::unique_ptr<BugType> BT_Exact, BT_Suspicious;
 
 public:
   void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
                         BugReporter &BR) const;
 
   void checkEndOfTranslationUnit(const TranslationUnitDecl *TU,
                                  AnalysisManager &Mgr, BugReporter &BR) const;
 
   /// \brief Reports all clones to the user.
-  void reportClones(SourceManager &SM, AnalysisManager &Mgr,
+  void reportClones(BugReporter &BR, AnalysisManager &Mgr,
                     int MinComplexity) const;
 
   /// \brief Reports only suspicious clones to the user along with informaton
   ///        that explain why they are suspicious.
-  void reportSuspiciousClones(SourceManager &SM, AnalysisManager &Mgr,
+  void reportSuspiciousClones(BugReporter &BR, AnalysisManager &Mgr,
                               int MinComplexity) const;
 };
 } // end anonymous namespace
 
 void CloneChecker::checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
                                     BugReporter &BR) const {
   // Every statement that should be included in the search for clones needs to
   // be passed to the CloneDetector.
void CloneChecker::checkEndOfTranslationUnit(const TranslationUnitDecl *TU,
 
   bool ReportSuspiciousClones = Mgr.getAnalyzerOptions().getBooleanOption(
       "ReportSuspiciousClones", true, this);
 
   bool ReportNormalClones = Mgr.getAnalyzerOptions().getBooleanOption(
       "ReportNormalClones", true, this);
 
   if (ReportSuspiciousClones)
-    reportSuspiciousClones(BR.getSourceManager(), Mgr, MinComplexity);
+    reportSuspiciousClones(BR, Mgr, MinComplexity);
 
   if (ReportNormalClones)
-    reportClones(BR.getSourceManager(), Mgr, MinComplexity);
+    reportClones(BR, Mgr, MinComplexity);
 }
 
-void CloneChecker::reportClones(SourceManager &SM, AnalysisManager &Mgr,
+static PathDiagnosticLocation makeLocation(const StmtSequence &S,
+                                           AnalysisManager &Mgr) {
+  ASTContext &ACtx = Mgr.getASTContext();
+  return PathDiagnosticLocation::createBegin(
+      S.front(), ACtx.getSourceManager(),
+      Mgr.getAnalysisDeclContext(ACtx.getTranslationUnitDecl()));
+}
+
+void CloneChecker::reportClones(BugReporter &BR, AnalysisManager &Mgr,
                                 int MinComplexity) const {
 
   std::vector<CloneDetector::CloneGroup> CloneGroups;
   Detector.findClones(CloneGroups, MinComplexity);
 
-  DiagnosticsEngine &DiagEngine = Mgr.getDiagnostic();
-
-  unsigned WarnID = DiagEngine.getCustomDiagID(DiagnosticsEngine::Warning,
-                                               "Detected code clone.");
-
-  unsigned NoteID = DiagEngine.getCustomDiagID(DiagnosticsEngine::Note,
-                                               "Related code clone is here.");
+  if (!BT_Exact)
+    BT_Exact.reset(new BugType(this, "Exact code clone", "Code clone"));
 
   for (CloneDetector::CloneGroup &Group : CloneGroups) {
     // We group the clones by printing the first as a warning and all others
     // as a note.
-    DiagEngine.Report(Group.Sequences.front().getStartLoc(), WarnID);
-    for (unsigned i = 1; i < Group.Sequences.size(); ++i) {
-      DiagEngine.Report(Group.Sequences[i].getStartLoc(), NoteID);
-    }
+    auto R = llvm::make_unique<BugReport>(
+        *BT_Exact, "Duplicate code detected",
+        makeLocation(Group.Sequences.front(), Mgr));
+    R->addRange(Group.Sequences.front().getSourceRange());
+
+    for (unsigned i = 1; i < Group.Sequences.size(); ++i)
+      R->addNote("Similar code here",
+                      makeLocation(Group.Sequences[i], Mgr),
+                      Group.Sequences[i].getSourceRange());
+    BR.emitReport(std::move(R));
   }
 }
 
-void CloneChecker::reportSuspiciousClones(SourceManager &SM,
+void CloneChecker::reportSuspiciousClones(BugReporter &BR,
                                           AnalysisManager &Mgr,
                                           int MinComplexity) const {
 
   std::vector<CloneDetector::SuspiciousClonePair> Clones;
   Detector.findSuspiciousClones(Clones, MinComplexity);
 
-  DiagnosticsEngine &DiagEngine = Mgr.getDiagnostic();
-
-  auto SuspiciousCloneWarning = DiagEngine.getCustomDiagID(
-      DiagnosticsEngine::Warning, "suspicious code clone detected; did you "
-                                  "mean to use %0?");
-
-  auto RelatedCloneNote = DiagEngine.getCustomDiagID(
-      DiagnosticsEngine::Note, "suggestion is based on the usage of this "
-                               "variable in a similar piece of code");
-
-  auto RelatedSuspiciousCloneNote = DiagEngine.getCustomDiagID(
-      DiagnosticsEngine::Note, "suggestion is based on the usage of this "
-                               "variable in a similar piece of code; did you "
-                               "mean to use %0?");
+  if (!BT_Suspicious)
+    BT_Suspicious.reset(
+        new BugType(this, "Suspicious code clone", "Code clone"));
+
+  ASTContext &ACtx = BR.getContext();
+  SourceManager &SM = ACtx.getSourceManager();
+  AnalysisDeclContext *ADC =
+      Mgr.getAnalysisDeclContext(ACtx.getTranslationUnitDecl());
 
   for (CloneDetector::SuspiciousClonePair &Pair : Clones) {
-    // The first clone always has a suggestion and we report it to the user
-    // along with the place where the suggestion should be used.
-    DiagEngine.Report(Pair.FirstCloneInfo.VarRange.getBegin(),
-                      SuspiciousCloneWarning)
-        << Pair.FirstCloneInfo.VarRange << Pair.FirstCloneInfo.Suggestion;
-
-    // The second clone can have a suggestion and if there is one, we report
-    // that suggestion to the user.
-    if (Pair.SecondCloneInfo.Suggestion) {
-      DiagEngine.Report(Pair.SecondCloneInfo.VarRange.getBegin(),
-                        RelatedSuspiciousCloneNote)
-          << Pair.SecondCloneInfo.VarRange << Pair.SecondCloneInfo.Suggestion;
-      continue;
-    }
-
-    // If there isn't a suggestion in the second clone, we only inform the
-    // user where we got the idea that his code could contain an error.
-    DiagEngine.Report(Pair.SecondCloneInfo.VarRange.getBegin(),
-                      RelatedCloneNote)
-        << Pair.SecondCloneInfo.VarRange;
+    // FIXME: We are ignoring the suggestions currently, because they are
+    // only 50% accurate (even if the second suggestion is unavailable),
+    // which may confuse the user.
+    // Think how to perform more accurate suggestions?
+
+    auto R = llvm::make_unique<BugReport>(
+        *BT_Suspicious,
+        "Potential copy-paste error; did you really mean to use '" +
+            Pair.FirstCloneInfo.Variable->getNameAsString() + "' here?",
+        PathDiagnosticLocation::createBegin(Pair.FirstCloneInfo.Mention, SM,
+                                            ADC));
+    R->addRange(Pair.FirstCloneInfo.Mention->getSourceRange());
+
+    R->addNote("Similar code using '" +
+                   Pair.SecondCloneInfo.Variable->getNameAsString() + "' here",
+               PathDiagnosticLocation::createBegin(Pair.SecondCloneInfo.Mention,
+                                                   SM, ADC),
+               Pair.SecondCloneInfo.Mention->getSourceRange());
+
+    BR.emitReport(std::move(R));
   }
 }
 
 //===----------------------------------------------------------------------===//
 // Register CloneChecker
 //===----------------------------------------------------------------------===//
 
 void ento::registerCloneChecker(CheckerManager &Mgr) {
   Mgr.registerChecker<CloneChecker>();
 }

cfe/trunk/test/Analysis/copypaste/blocks.cpp

 // RUN: %clang_cc1 -analyze -fblocks -std=c++11 -analyzer-checker=alpha.clone.CloneChecker -verify %s
 
 // This tests if we search for clones in blocks.
 
 void log();
 
-auto BlockA = ^(int a, int b){ // expected-warning{{Detected code clone.}}
+auto BlockA = ^(int a, int b){ // expected-warning{{Duplicate code detected}}
   log();
   if (a > b)
     return a;
   return b;
 };
 
-auto BlockB = ^(int a, int b){ // expected-note{{Related code clone is here.}}
+auto BlockB = ^(int a, int b){ // expected-note{{Similar code here}}
   log();
   if (a > b)
     return a;
   return b;
 };

cfe/trunk/test/Analysis/copypaste/function-try-block.cpp

 // RUN: %clang_cc1 -analyze -fcxx-exceptions -std=c++1z -analyzer-checker=alpha.clone.CloneChecker -verify %s
 
 // Tests if function try blocks are correctly handled.
 
 void nonCompoundStmt1(int& x)
-  try { x += 1; } catch(...) { x -= 1; } // expected-warning{{Detected code clone.}}
+  try { x += 1; } catch(...) { x -= 1; } // expected-warning{{Duplicate code detected}}
 
 void nonCompoundStmt2(int& x)
-  try { x += 1; } catch(...) { x -= 1; } // expected-note{{Related code clone is here.}}
+  try { x += 1; } catch(...) { x -= 1; } // expected-note{{Similar code here}}

cfe/trunk/test/Analysis/copypaste/functions.cpp

 // RUN: %clang_cc1 -analyze -std=c++11 -analyzer-checker=alpha.clone.CloneChecker -verify %s
 
 // This tests if we search for clones in functions.
 
 void log();
 
-int max(int a, int b) { // expected-warning{{Detected code clone.}}
+int max(int a, int b) { // expected-warning{{Duplicate code detected}}
   log();
   if (a > b)
     return a;
   return b;
 }
 
-int maxClone(int x, int y) { // expected-note{{Related code clone is here.}}
+int maxClone(int x, int y) { // expected-note{{Similar code here}}
   log();
   if (x > y)
     return x;
   return y;
 }
 
 // Functions below are not clones and should not be reported.
 

cfe/trunk/test/Analysis/copypaste/macro-complexity.cpp

 // RUN: %clang_cc1 -analyze -std=c++11 -analyzer-checker=alpha.clone.CloneChecker -analyzer-config alpha.clone.CloneChecker:MinimumCloneComplexity=10 -verify %s
 
 // Tests that the complexity value of a macro expansion is about the same as
 // the complexity value of a normal function call and the the macro body doesn't
 // influence the complexity. See the CloneSignature class in CloneDetection.h
 // for more information about complexity values of clones.
 
 #define MACRO_FOO(a, b) a > b ? -a * a : -b * b;
 
 // First, manually apply MACRO_FOO and see if the code gets detected as a clone.
 // This confirms that with the current configuration the macro body would be
 // considered large enough to pass the MinimumCloneComplexity constraint.
 
-int manualMacro(int a, int b) { // expected-warning{{Detected code clone.}}
+int manualMacro(int a, int b) { // expected-warning{{Duplicate code detected}}
   return a > b ? -a * a : -b * b;
 }
 
-int manualMacroClone(int a, int b) { // expected-note{{Related code clone is here.}}
+int manualMacroClone(int a, int b) { // expected-note{{Similar code here}}
   return a > b ? -a * a : -b * b;
 }
 
 // Now we actually use the macro to generate the same AST as above. They
 // shouldn't be reported because the macros only slighly increase the complexity
 // value and the resulting code will never pass the MinimumCloneComplexity
 // constraint.
 

 // than normal code. We also need to be sure this amount is not zero because
 // we otherwise macro code would be 'invisible' for the CloneDetector.
 // This tests that it is possible to increase the reach the minimum complexity
 // by only using macros. This is only possible if the complexity value is bigger
 // than zero.
 
 #define NEG(A) -(A)
 
-int nestedMacros() { // expected-warning{{Detected code clone.}}
+int nestedMacros() { // expected-warning{{Duplicate code detected}}
   return NEG(NEG(NEG(NEG(NEG(NEG(NEG(NEG(NEG(NEG(1))))))))));
 }
 
-int nestedMacrosClone() { // expected-note{{Related code clone is here.}}
+int nestedMacrosClone() { // expected-note{{Similar code here}}
   return NEG(NEG(NEG(NEG(NEG(NEG(NEG(NEG(NEG(NEG(1))))))))));
 }

cfe/trunk/test/Analysis/copypaste/macros.cpp

 // RUN: %clang_cc1 -analyze -std=c++11 -analyzer-checker=alpha.clone.CloneChecker -verify %s
 
 // Tests that macros and non-macro clones aren't mixed into the same hash
 // group. This is currently necessary as all clones in a hash group need
 // to have the same complexity value. Macros have smaller complexity values
 // and need to be in their own hash group.
 
-int foo(int a) { // expected-warning{{Detected code clone.}}
+int foo(int a) { // expected-warning{{Duplicate code detected}}
   a = a + 1;
   a = a + 1 / 1;
   a = a + 1 + 1 + 1;
   a = a + 1 - 1 + 1 + 1;
   a = a + 1 * 1 + 1 + 1 + 1;
   a = a + 1 / 1 + 1 + 1 + 1;
   return a;
 }
 
-int fooClone(int a) { // expected-note{{Related code clone is here.}}
+int fooClone(int a) { // expected-note{{Similar code here}}
   a = a + 1;
   a = a + 1 / 1;
   a = a + 1 + 1 + 1;
   a = a + 1 - 1 + 1 + 1;
   a = a + 1 * 1 + 1 + 1 + 1;
   a = a + 1 / 1 + 1 + 1 + 1;
   return a;
 }
 
 // Below is the same AST as above but this time generated with macros. The
 // clones below should land in their own hash group for the reasons given above.
 
 #define ASSIGN(T, V) T = T + V
 
-int macro(int a) { // expected-warning{{Detected code clone.}}
+int macro(int a) { // expected-warning{{Duplicate code detected}}
   ASSIGN(a, 1);
   ASSIGN(a, 1 / 1);
   ASSIGN(a, 1 + 1 + 1);
   ASSIGN(a, 1 - 1 + 1 + 1);
   ASSIGN(a, 1 * 1 + 1 + 1 + 1);
   ASSIGN(a, 1 / 1 + 1 + 1 + 1);
   return a;
 }
 
-int macroClone(int a) { // expected-note{{Related code clone is here.}}
+int macroClone(int a) { // expected-note{{Similar code here}}
   ASSIGN(a, 1);
   ASSIGN(a, 1 / 1);
   ASSIGN(a, 1 + 1 + 1);
   ASSIGN(a, 1 - 1 + 1 + 1);
   ASSIGN(a, 1 * 1 + 1 + 1 + 1);
   ASSIGN(a, 1 / 1 + 1 + 1 + 1);
   return a;
 }
 
 // FIXME: Macros with empty definitions in the AST are currently ignored.
 
 #define EMPTY
 
-int fooFalsePositiveClone(int a) { // expected-note{{Related code clone is here.}}
+int fooFalsePositiveClone(int a) { // expected-note{{Similar code here}}
   a = EMPTY a + 1;
   a = a + 1 / 1;
   a = a + 1 + 1 + 1;
   a = a + 1 - 1 + 1 + 1;
   a = a + 1 * 1 + 1 + 1 + 1;
   a = a + 1 / 1 + 1 + 1 + 1;
   return a;
 }
 
 

cfe/trunk/test/Analysis/copypaste/objc-methods.m

 // RUN: %clang_cc1 -analyze -Wno-objc-root-class -analyzer-checker=alpha.clone.CloneChecker -verify %s
 
 // This tests if we search for clones in Objective-C methods.
 
 @interface A
 - (int) setOk : (int) a : (int) b;
 @end
 
 @implementation A
-- (int) setOk : (int) a : (int) b {  // expected-warning{{Detected code clone.}}
+- (int) setOk : (int) a : (int) b {  // expected-warning{{Duplicate code detected}}
   if (a > b)
     return a;
   return b;
 }
 @end
 
 @interface B
 - (int) setOk : (int) a : (int) b;
 @end
 
 @implementation B
-- (int) setOk : (int) a : (int) b { // expected-note{{Related code clone is here.}}
+- (int) setOk : (int) a : (int) b { // expected-note{{Similar code here}}
   if (a > b)
     return a;
   return b;
 }
 @end

cfe/trunk/test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp

+// RUN: %clang_cc1 -analyze -analyzer-output=plist -analyzer-config notes-as-events=true -o %t.plist -std=c++11 -analyzer-checker=alpha.clone.CloneChecker -verify %s
+// RUN: FileCheck --input-file=%t.plist %s
+
+void log();
+
+int max(int a, int b) { // expected-warning{{Duplicate code detected}}
+  log();
+  if (a > b)
+    return a;
+  return b;
+}
+
+int maxClone(int a, int b) { // no-note (converted into event)
+  log();
+  if (a > b)
+    return a;
+  return b;
+}
+
+// CHECK:  <key>diagnostics</key>
+// CHECK-NEXT:  <array>
+// CHECK-NEXT:   <dict>
+// CHECK-NEXT:    <key>path</key>
+// CHECK-NEXT:    <array>
+// CHECK-NEXT:     <dict>
+// CHECK-NEXT:      <key>kind</key><string>event</string>
+// CHECK-NEXT:      <key>location</key>
+// CHECK-NEXT:      <dict>
+// CHECK-NEXT:       <key>line</key><integer>13</integer>
+// CHECK-NEXT:       <key>col</key><integer>28</integer>
+// CHECK-NEXT:       <key>file</key><integer>0</integer>
+// CHECK-NEXT:      </dict>
+// CHECK-NEXT:      <key>ranges</key>
+// CHECK-NEXT:      <array>
+// CHECK-NEXT:        <array>
+// CHECK-NEXT:         <dict>
+// CHECK-NEXT:          <key>line</key><integer>13</integer>
+// CHECK-NEXT:          <key>col</key><integer>28</integer>
+// CHECK-NEXT:          <key>file</key><integer>0</integer>
+// CHECK-NEXT:         </dict>
+// CHECK-NEXT:         <dict>
+// CHECK-NEXT:          <key>line</key><integer>18</integer>
+// CHECK-NEXT:          <key>col</key><integer>1</integer>
+// CHECK-NEXT:          <key>file</key><integer>0</integer>
+// CHECK-NEXT:         </dict>
+// CHECK-NEXT:        </array>
+// CHECK-NEXT:      </array>
+// CHECK-NEXT:      <key>depth</key><integer>0</integer>
+// CHECK-NEXT:      <key>extended_message</key>
+// CHECK-NEXT:      <string>Similar code here</string>
+// CHECK-NEXT:      <key>message</key>
+// CHECK-NEXT:      <string>Similar code here</string>
+// CHECK-NEXT:     </dict>
+// CHECK-NEXT:     <dict>
+// CHECK-NEXT:      <key>kind</key><string>event</string>
+// CHECK-NEXT:      <key>location</key>
+// CHECK-NEXT:      <dict>
+// CHECK-NEXT:       <key>line</key><integer>6</integer>
+// CHECK-NEXT:       <key>col</key><integer>23</integer>
+// CHECK-NEXT:       <key>file</key><integer>0</integer>
+// CHECK-NEXT:      </dict>
+// CHECK-NEXT:      <key>ranges</key>
+// CHECK-NEXT:      <array>
+// CHECK-NEXT:        <array>
+// CHECK-NEXT:         <dict>
+// CHECK-NEXT:          <key>line</key><integer>6</integer>
+// CHECK-NEXT:          <key>col</key><integer>23</integer>
+// CHECK-NEXT:          <key>file</key><integer>0</integer>
+// CHECK-NEXT:         </dict>
+// CHECK-NEXT:         <dict>
+// CHECK-NEXT:          <key>line</key><integer>11</integer>
+// CHECK-NEXT:          <key>col</key><integer>1</integer>
+// CHECK-NEXT:          <key>file</key><integer>0</integer>
+// CHECK-NEXT:         </dict>
+// CHECK-NEXT:        </array>
+// CHECK-NEXT:      </array>
+// CHECK-NEXT:      <key>depth</key><integer>0</integer>
+// CHECK-NEXT:      <key>extended_message</key>
+// CHECK-NEXT:      <string>Duplicate code detected</string>
+// CHECK-NEXT:      <key>message</key>
+// CHECK-NEXT:      <string>Duplicate code detected</string>
+// CHECK-NEXT:     </dict>
+// CHECK-NEXT:    </array>
+// CHECK-NEXT:    <key>description</key><string>Duplicate code detected</string>
+// CHECK-NEXT:    <key>category</key><string>Code clone</string>
+// CHECK-NEXT:    <key>type</key><string>Exact code clone</string>
+// CHECK-NEXT:    <key>check_name</key><string>alpha.clone.CloneChecker</string>
+// CHECK-NEXT:    <!-- This hash is experimental and going to change! -->
+// CHECK-NEXT:    <key>issue_hash_content_of_line_in_context</key><string>3d15184f38c5fa57e479b744fe3f5035</string>
+// CHECK-NEXT:   <key>location</key>
+// CHECK-NEXT:   <dict>
+// CHECK-NEXT:    <key>line</key><integer>6</integer>
+// CHECK-NEXT:    <key>col</key><integer>23</integer>
+// CHECK-NEXT:    <key>file</key><integer>0</integer>
+// CHECK-NEXT:   </dict>
+// CHECK-NEXT:   </dict>
+// CHECK-NEXT:  </array>

cfe/trunk/test/Analysis/copypaste/plist-diagnostics.cpp

+// RUN: %clang_cc1 -analyze -analyzer-output=plist -o %t.plist -std=c++11 -analyzer-checker=alpha.clone.CloneChecker -verify %s
+// RUN: FileCheck --input-file=%t.plist %s
+
+void log();
+
+int max(int a, int b) { // expected-warning{{Duplicate code detected}}
+  log();
+  if (a > b)
+    return a;
+  return b;
+}
+
+int maxClone(int a, int b) { // expected-note{{Similar code here}}
+  log();
+  if (a > b)
+    return a;
+  return b;
+}
+
+// FIXME: This plist output doesn't include the extra note on line 13.
+// It should be updated once the format for extra notes in plists is defined.
+
+// CHECK:  <key>diagnostics</key>
+// CHECK-NEXT:  <array>
+// CHECK-NEXT:   <dict>
+// CHECK-NEXT:    <key>path</key>
+// CHECK-NEXT:    <array>
+// CHECK-NEXT:     <dict>
+// CHECK-NEXT:      <key>kind</key><string>event</string>
+// CHECK-NEXT:      <key>location</key>
+// CHECK-NEXT:      <dict>
+// CHECK-NEXT:       <key>line</key><integer>6</integer>
+// CHECK-NEXT:       <key>col</key><integer>23</integer>
+// CHECK-NEXT:       <key>file</key><integer>0</integer>
+// CHECK-NEXT:      </dict>
+// CHECK-NEXT:      <key>ranges</key>
+// CHECK-NEXT:      <array>
+// CHECK-NEXT:        <array>
+// CHECK-NEXT:         <dict>
+// CHECK-NEXT:          <key>line</key><integer>6</integer>
+// CHECK-NEXT:          <key>col</key><integer>23</integer>
+// CHECK-NEXT:          <key>file</key><integer>0</integer>
+// CHECK-NEXT:         </dict>
+// CHECK-NEXT:         <dict>
+// CHECK-NEXT:          <key>line</key><integer>11</integer>
+// CHECK-NEXT:          <key>col</key><integer>1</integer>
+// CHECK-NEXT:          <key>file</key><integer>0</integer>
+// CHECK-NEXT:         </dict>
+// CHECK-NEXT:        </array>
+// CHECK-NEXT:      </array>
+// CHECK-NEXT:      <key>depth</key><integer>0</integer>
+// CHECK-NEXT:      <key>extended_message</key>
+// CHECK-NEXT:      <string>Duplicate code detected</string>
+// CHECK-NEXT:      <key>message</key>
+// CHECK-NEXT:      <string>Duplicate code detected</string>
+// CHECK-NEXT:     </dict>
+// CHECK-NEXT:    </array>
+// CHECK-NEXT:    <key>description</key><string>Duplicate code detected</string>
+// CHECK-NEXT:    <key>category</key><string>Code clone</string>
+// CHECK-NEXT:    <key>type</key><string>Exact code clone</string>
+// CHECK-NEXT:    <key>check_name</key><string>alpha.clone.CloneChecker</string>
+// CHECK-NEXT:    <!-- This hash is experimental and going to change! -->
+// CHECK-NEXT:    <key>issue_hash_content_of_line_in_context</key><string>3d15184f38c5fa57e479b744fe3f5035</string>
+// CHECK-NEXT:   <key>location</key>
+// CHECK-NEXT:   <dict>
+// CHECK-NEXT:    <key>line</key><integer>6</integer>
+// CHECK-NEXT:    <key>col</key><integer>23</integer>
+// CHECK-NEXT:    <key>file</key><integer>0</integer>
+// CHECK-NEXT:   </dict>
+// CHECK-NEXT:   </dict>
+// CHECK-NEXT:  </array>

cfe/trunk/test/Analysis/copypaste/sub-sequences.cpp

 // RUN: %clang_cc1 -analyze -std=c++11 -analyzer-checker=alpha.clone.CloneChecker -verify %s
 
 // This tests if sub-sequences can match with normal sequences.
 
 void log2(int a);
 void log();
 
 int max(int a, int b) {
   log2(a);
-  log(); // expected-warning{{Detected code clone.}}
+  log(); // expected-warning{{Duplicate code detected}}
   if (a > b)
     return a;
   return b;
 }
 
 int maxClone(int a, int b) {
-  log(); // expected-note{{Related code clone is here.}}
+  log(); // expected-note{{Similar code here}}
   if (a > b)
     return a;
   return b;
 }
 
 // Functions below are not clones and should not be reported.
 
 int foo(int a, int b) { // no-warning
   return a + b;
 }

cfe/trunk/test/Analysis/copypaste/suspicious-clones.cpp

 // RUN: %clang_cc1 -analyze -analyzer-checker=alpha.clone.CloneChecker -analyzer-config alpha.clone.CloneChecker:ReportSuspiciousClones=true  -analyzer-config alpha.clone.CloneChecker:ReportNormalClones=false -verify %s
 
 // Tests finding a suspicious clone that references local variables.
 
 void log();
 
 int max(int a, int b) {
   log();
   if (a > b)
     return a;
-  return b; // expected-note{{suggestion is based on the usage of this variable in a similar piece of code}}
+  return b; // expected-note{{Similar code using 'b' here}}
 }
 
 int maxClone(int x, int y, int z) {
   log();
   if (x > y)
     return x;
-  return z; // expected-warning{{suspicious code clone detected; did you mean to use 'y'?}}
+  return z; // expected-warning{{Potential copy-paste error; did you really mean to use 'z' here?}}
 }
 
 // Tests finding a suspicious clone that references global variables.
 
 struct mutex {
   bool try_lock();
   void unlock();
 };
 
 mutex m1;
 mutex m2;
 int i;
 
 void busyIncrement() {
   while (true) {
     if (m1.try_lock()) {
       ++i;
-      m1.unlock(); // expected-note{{suggestion is based on the usage of this variable in a similar piece of code}}
+      m1.unlock(); // expected-note{{Similar code using 'm1' here}}
       if (i > 1000) {
         return;
       }
     }
   }
 }
 
 void faultyBusyIncrement() {
   while (true) {
     if (m1.try_lock()) {
       ++i;
-      m2.unlock();  // expected-warning{{suspicious code clone detected; did you mean to use 'm1'?}}
+      m2.unlock();  // expected-warning{{Potential copy-paste error; did you really mean to use 'm2' here?}}
       if (i > 1000) {
         return;
       }
     }
   }
 }
 
 // Tests that we provide two suggestions in cases where two fixes are possible.
 
 int foo(int a, int b, int c) {
   a += b + c;
   b /= a + b;
-  c -= b * a; // expected-warning{{suspicious code clone detected; did you mean to use 'a'?}}
+  c -= b * a; // expected-warning{{Potential copy-paste error; did you really mean to use 'b' here?}}
   return c;
 }
 
 int fooClone(int a, int b, int c) {
   a += b + c;
   b /= a + b;
-  c -= a * a; // expected-note{{suggestion is based on the usage of this variable in a similar piece of code; did you mean to use 'b'?}}
+  c -= a * a; // expected-note{{Similar code using 'a' here}}
   return c;
 }
 
 
 // Tests that for clone groups with a many possible suspicious clone pairs, at
 // most one warning per clone group is generated and every relevant clone is
 // reported through either a warning or a note.
 
 long bar1(long a, long b, long c, long d) {
   c = a - b;
   c = c / d * a;
-  d = b * b - c; // expected-warning{{suspicious code clone detected; did you mean to use 'c'?}}
+  d = b * b - c; // expected-warning{{Potential copy-paste error; did you really mean to use 'b' here?}}
   return d;
 }
 
 long bar2(long a, long b, long c, long d) {
   c = a - b;
   c = c / d * a;
-  d = c * b - c; // expected-note{{suggestion is based on the usage of this variable in a similar piece of code; did you mean to use 'b'?}} \
-                 // expected-warning{{suspicious code clone detected; did you mean to use 'a'?}}
+  d = c * b - c; // expected-note{{Similar code using 'c' here}} \
+                 // expected-warning{{Potential copy-paste error; did you really mean to use 'c' here?}}
   return d;
 }
 
 long bar3(long a, long b, long c, long d) {
   c = a - b;
   c = c / d * a;
-  d = a * b - c; // expected-note{{suggestion is based on the usage of this variable in a similar piece of code; did you mean to use 'c'?}}
+  d = a * b - c; // expected-note{{Similar code using 'a' here}}
   return d;
 }

cfe/trunk/test/Analysis/copypaste/text-diagnostics.cpp

+// RUN: %clang_cc1 -analyze -analyzer-output=text -std=c++11 -analyzer-checker=alpha.clone.CloneChecker -verify %s
+
+void log();
+
+int max(int a, int b) { // expected-warning{{Duplicate code detected}} // expected-note{{Duplicate code detected}}
+  log();
+  if (a > b)
+    return a;
+  return b;
+}
+
+int maxClone(int a, int b) { // expected-note{{Similar code here}}
+  log();
+  if (a > b)
+    return a;
+  return b;
+}