Merge remote-tracking branch 'llvm.org/master' into apple/master

Author: rjmccall

clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp

@@ -23,6 +23,7 @@
 #include "DontModifyStdNamespaceCheck.h"
 #include "FloatLoopCounter.h"
 #include "LimitedRandomnessCheck.h"
+#include "MutatingCopyCheck.h"
 #include "PostfixOperatorCheck.h"
 #include "ProperlySeededRandomGeneratorCheck.h"
 #include "SetLongJmpCheck.h"
@@ -69,6 +70,8 @@ class CERTModule : public ClangTidyModule {
         "cert-oop11-cpp");
     CheckFactories.registerCheck<bugprone::UnhandledSelfAssignmentCheck>(
         "cert-oop54-cpp");
+    CheckFactories.registerCheck<MutatingCopyCheck>(
+        "cert-oop58-cpp");
 
     // C checkers
     // DCL

clang-tools-extra/clang-tidy/cert/CMakeLists.txt

@@ -7,6 +7,7 @@ add_clang_library(clangTidyCERTModule
   DontModifyStdNamespaceCheck.cpp
   FloatLoopCounter.cpp
   LimitedRandomnessCheck.cpp
+  MutatingCopyCheck.cpp
   PostfixOperatorCheck.cpp
   ProperlySeededRandomGeneratorCheck.cpp
   SetLongJmpCheck.cpp

clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.cpp

@@ -0,0 +1,83 @@
+//===--- MutatingCopyCheck.cpp - clang-tidy -------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "MutatingCopyCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cert {
+
+static constexpr llvm::StringLiteral SourceDeclName = "ChangedPVD";
+static constexpr llvm::StringLiteral MutatingOperatorName = "MutatingOp";
+static constexpr llvm::StringLiteral MutatingCallName = "MutatingCall";
+
+void MutatingCopyCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  const auto MemberExprOrSourceObject = anyOf(
+      memberExpr(), declRefExpr(to(decl(equalsBoundNode(SourceDeclName)))));
+
+  const auto IsPartOfSource =
+      allOf(unless(hasDescendant(expr(unless(MemberExprOrSourceObject)))),
+            MemberExprOrSourceObject);
+
+  const auto IsSourceMutatingAssignment =
+      expr(anyOf(binaryOperator(isAssignmentOperator(), hasLHS(IsPartOfSource))
+                     .bind(MutatingOperatorName),
+                 cxxOperatorCallExpr(isAssignmentOperator(),
+                                     hasArgument(0, IsPartOfSource))
+                     .bind(MutatingOperatorName)));
+
+  const auto MemberExprOrSelf = anyOf(memberExpr(), cxxThisExpr());
+
+  const auto IsPartOfSelf = allOf(
+      unless(hasDescendant(expr(unless(MemberExprOrSelf)))), MemberExprOrSelf);
+
+  const auto IsSelfMutatingAssignment =
+      expr(anyOf(binaryOperator(isAssignmentOperator(), hasLHS(IsPartOfSelf)),
+                 cxxOperatorCallExpr(isAssignmentOperator(),
+                                     hasArgument(0, IsPartOfSelf))));
+
+  const auto IsSelfMutatingMemberFunction =
+      functionDecl(hasBody(hasDescendant(IsSelfMutatingAssignment)));
+
+  const auto IsSourceMutatingMemberCall =
+      cxxMemberCallExpr(on(IsPartOfSource),
+                        callee(IsSelfMutatingMemberFunction))
+          .bind(MutatingCallName);
+
+  const auto MutatesSource = allOf(
+      hasParameter(
+          0, parmVarDecl(hasType(lValueReferenceType())).bind(SourceDeclName)),
+      anyOf(forEachDescendant(IsSourceMutatingAssignment),
+            forEachDescendant(IsSourceMutatingMemberCall)));
+
+  Finder->addMatcher(cxxConstructorDecl(isCopyConstructor(), MutatesSource),
+                     this);
+
+  Finder->addMatcher(cxxMethodDecl(isCopyAssignmentOperator(), MutatesSource),
+                     this);
+}
+
+void MutatingCopyCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *MemberCall =
+          Result.Nodes.getNodeAs<CXXMemberCallExpr>(MutatingCallName))
+    diag(MemberCall->getBeginLoc(), "call mutates copied object");
+  else if (const auto *Assignment =
+               Result.Nodes.getNodeAs<Expr>(MutatingOperatorName))
+    diag(Assignment->getBeginLoc(), "mutating copied object");
+}
+
+} // namespace cert
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.h

@@ -0,0 +1,35 @@
+//===--- MutatingCopyCheck.h - clang-tidy -----------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_MUTATINGCOPYCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_MUTATINGCOPYCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace cert {
+
+/// Finds assignments to the copied object and its direct or indirect members
+/// in copy constructors and copy assignment operators.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cert-oop58-cpp.html
+class MutatingCopyCheck : public ClangTidyCheck {
+public:
+  MutatingCopyCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace cert
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_MUTATINGCOPYCHECK_H

clang-tools-extra/clangd/AST.cpp

@@ -377,7 +377,7 @@ class DeducedTypeVisitor : public RecursiveASTVisitor<DeducedTypeVisitor> {
     // Loc of "auto" in operator auto()
     if (CurLoc.isInvalid() && dyn_cast<CXXConversionDecl>(D))
       CurLoc = D->getTypeSourceInfo()->getTypeLoc().getBeginLoc();
-    // Loc of "auto" in function with traling return type (c++11).
+    // Loc of "auto" in function with trailing return type (c++11).
     if (CurLoc.isInvalid())
       CurLoc = D->getSourceRange().getBegin();
     if (CurLoc != SearchedLocation)

clang-tools-extra/clangd/ClangdLSPServer.cpp

@@ -566,10 +566,6 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params,
             {"declarationProvider", true},
             {"definitionProvider", true},
             {"documentHighlightProvider", true},
-            {"documentLinkProvider",
-             llvm::json::Object{
-                 {"resolveProvider", false},
-             }},
             {"hoverProvider", true},
             {"renameProvider", std::move(RenameProvider)},
             {"selectionRangeProvider", true},
@@ -1204,25 +1200,6 @@ void ClangdLSPServer::onSelectionRange(
       });
 }
 
-void ClangdLSPServer::onDocumentLink(
-    const DocumentLinkParams &Params,
-    Callback<std::vector<DocumentLink>> Reply) {
-
-  // TODO(forster): This currently resolves all targets eagerly. This is slow,
-  // because it blocks on the preamble/AST being built. We could respond to the
-  // request faster by using string matching or the lexer to find the includes
-  // and resolving the targets lazily.
-  Server->documentLinks(
-      Params.textDocument.uri.file(),
-      [Reply = std::move(Reply)](
-          llvm::Expected<std::vector<DocumentLink>> Links) mutable {
-        if (!Links) {
-          return Reply(Links.takeError());
-        }
-        return Reply(std::move(Links));
-      });
-}
-
 ClangdLSPServer::ClangdLSPServer(
     class Transport &Transp, const FileSystemProvider &FSProvider,
     const clangd::CodeCompleteOptions &CCOpts,
@@ -1266,7 +1243,6 @@ ClangdLSPServer::ClangdLSPServer(
   MsgHandler->bind("textDocument/typeHierarchy", &ClangdLSPServer::onTypeHierarchy);
   MsgHandler->bind("typeHierarchy/resolve", &ClangdLSPServer::onResolveTypeHierarchy);
   MsgHandler->bind("textDocument/selectionRange", &ClangdLSPServer::onSelectionRange);
-  MsgHandler->bind("textDocument/documentLink", &ClangdLSPServer::onDocumentLink);
   // clang-format on
 }
 

clang-tools-extra/clangd/ClangdLSPServer.h

@@ -111,8 +111,6 @@ class ClangdLSPServer : private DiagnosticsConsumer {
                     Callback<std::vector<SymbolDetails>>);
   void onSelectionRange(const SelectionRangeParams &,
                         Callback<std::vector<SelectionRange>>);
-  void onDocumentLink(const DocumentLinkParams &,
-                      Callback<std::vector<DocumentLink>>);
 
   std::vector<Fix> getFixes(StringRef File, const clangd::Diagnostic &D);
 

clang-tools-extra/clangd/ClangdServer.cpp

@@ -374,24 +374,15 @@ void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
   WorkScheduler.runWithAST("Rename", File, std::move(Action));
 }
 
-// May generate several candidate selections, due to SelectionTree ambiguity.
-static llvm::Expected<std::vector<Tweak::Selection>>
+static llvm::Expected<Tweak::Selection>
 tweakSelection(const Range &Sel, const InputsAndAST &AST) {
   auto Begin = positionToOffset(AST.Inputs.Contents, Sel.start);
   if (!Begin)
     return Begin.takeError();
   auto End = positionToOffset(AST.Inputs.Contents, Sel.end);
   if (!End)
     return End.takeError();
-  std::vector<Tweak::Selection> Result;
-  SelectionTree::createEach(AST.AST.getASTContext(), AST.AST.getTokens(),
-                            *Begin, *End, [&](SelectionTree T) {
-                              Result.emplace_back(AST.Inputs.Index, AST.AST,
-                                                  *Begin, *End, std::move(T));
-                              return false;
-                            });
-  assert(!Result.empty() && "Expected at least one SelectionTree");
-  return Result;
+  return Tweak::Selection(AST.Inputs.Index, AST.AST, *Begin, *End);
 }
 
 void ClangdServer::enumerateTweaks(PathRef File, Range Sel,
@@ -400,21 +391,12 @@ void ClangdServer::enumerateTweaks(PathRef File, Range Sel,
                  this](Expected<InputsAndAST> InpAST) mutable {
     if (!InpAST)
       return CB(InpAST.takeError());
-    auto Selections = tweakSelection(Sel, *InpAST);
-    if (!Selections)
-      return CB(Selections.takeError());
+    auto Selection = tweakSelection(Sel, *InpAST);
+    if (!Selection)
+      return CB(Selection.takeError());
     std::vector<TweakRef> Res;
-    // Don't allow a tweak to fire more than once across ambiguous selections.
-    llvm::DenseSet<llvm::StringRef> PreparedTweaks;
-    auto Filter = [&](const Tweak &T) {
-      return TweakFilter(T) && !PreparedTweaks.count(T.id());
-    };
-    for (const auto &Sel : *Selections) {
-      for (auto &T : prepareTweaks(Sel, Filter)) {
-        Res.push_back({T->id(), T->title(), T->intent()});
-        PreparedTweaks.insert(T->id());
-      }
-    }
+    for (auto &T : prepareTweaks(*Selection, TweakFilter))
+      Res.push_back({T->id(), T->title(), T->intent()});
 
     CB(std::move(Res));
   };
@@ -429,30 +411,21 @@ void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
        FS = FSProvider.getFileSystem()](Expected<InputsAndAST> InpAST) mutable {
         if (!InpAST)
           return CB(InpAST.takeError());
-        auto Selections = tweakSelection(Sel, *InpAST);
-        if (!Selections)
-          return CB(Selections.takeError());
-        llvm::Optional<llvm::Expected<Tweak::Effect>> Effect;
-        // Try each selection, take the first one that prepare()s.
-        // If they all fail, Effect will hold get the last error.
-        for (const auto &Selection : *Selections) {
-          auto T = prepareTweak(TweakID, Selection);
-          if (T) {
-            Effect = (*T)->apply(Selection);
-            break;
-          }
-          Effect = T.takeError();
-        }
-        assert(Effect.hasValue() && "Expected at least one selection");
-        if (*Effect) {
-          // Tweaks don't apply clang-format, do that centrally here.
-          for (auto &It : (*Effect)->ApplyEdits) {
-            Edit &E = It.second;
-            format::FormatStyle Style =
-                getFormatStyleForFile(File, E.InitialCode, FS.get());
-            if (llvm::Error Err = reformatEdit(E, Style))
-              elog("Failed to format {0}: {1}", It.first(), std::move(Err));
-          }
+        auto Selection = tweakSelection(Sel, *InpAST);
+        if (!Selection)
+          return CB(Selection.takeError());
+        auto A = prepareTweak(TweakID, *Selection);
+        if (!A)
+          return CB(A.takeError());
+        auto Effect = (*A)->apply(*Selection);
+        if (!Effect)
+          return CB(Effect.takeError());
+        for (auto &It : Effect->ApplyEdits) {
+          Edit &E = It.second;
+          format::FormatStyle Style =
+              getFormatStyleForFile(File, E.InitialCode, FS.get());
+          if (llvm::Error Err = reformatEdit(E, Style))
+            elog("Failed to format {0}: {1}", It.first(), std::move(Err));
         }
         return CB(std::move(*Effect));
       };
@@ -493,7 +466,7 @@ void ClangdServer::locateSymbolAt(PathRef File, Position Pos,
 
 void ClangdServer::switchSourceHeader(
     PathRef Path, Callback<llvm::Optional<clangd::Path>> CB) {
-  // We want to return the result as fast as possible, stragety is:
+  // We want to return the result as fast as possible, strategy is:
   //  1) use the file-only heuristic, it requires some IO but it is much
   //     faster than building AST, but it only works when .h/.cc files are in
   //     the same directory.
@@ -638,17 +611,6 @@ void ClangdServer::semanticRanges(PathRef File, Position Pos,
   WorkScheduler.runWithAST("SemanticRanges", File, std::move(Action));
 }
 
-void ClangdServer::documentLinks(PathRef File,
-                                 Callback<std::vector<DocumentLink>> CB) {
-  auto Action =
-      [CB = std::move(CB)](llvm::Expected<InputsAndAST> InpAST) mutable {
-        if (!InpAST)
-          return CB(InpAST.takeError());
-        CB(clangd::getDocumentLinks(InpAST->AST));
-      };
-  WorkScheduler.runWithAST("DocumentLinks", File, std::move(Action));
-}
-
 std::vector<std::pair<Path, std::size_t>>
 ClangdServer::getUsedBytesPerFile() const {
   return WorkScheduler.getUsedBytesPerFile();

clang-tools-extra/clangd/ClangdServer.h

@@ -287,9 +287,6 @@ class ClangdServer {
   void semanticRanges(PathRef File, Position Pos,
                       Callback<std::vector<Range>> CB);
 
-  /// Get all document links in a file.
-  void documentLinks(PathRef File, Callback<std::vector<DocumentLink>> CB);
- 
   /// Returns estimated memory usage for each of the currently open files.
   /// The order of results is unspecified.
   /// Overall memory usage of clangd may be significantly more than reported

clang-tools-extra/clangd/CodeComplete.cpp

@@ -395,7 +395,7 @@ struct CodeCompletionBuilder {
     std::string ReturnType;
   };
 
-  // If all BundledEntrys have the same value for a property, return it.
+  // If all BundledEntries have the same value for a property, return it.
   template <std::string BundledEntry::*Member>
   const std::string *onlyValue() const {
     auto B = Bundled.begin(), E = Bundled.end();
@@ -499,7 +499,7 @@ llvm::Optional<SymbolID> getSymbolID(const CodeCompletionResult &R,
   llvm_unreachable("unknown CodeCompletionResult kind");
 }
 
-// Scopes of the paritial identifier we're trying to complete.
+// Scopes of the partial identifier we're trying to complete.
 // It is used when we query the index for more completion results.
 struct SpecifiedScope {
   // The scopes we should look in, determined by Sema.
@@ -874,7 +874,7 @@ class SignatureHelpCollector final : public CodeCompleteConsumer {
       // Function Template.
       // - High score is better.
       // - Shorter signature is better.
-      // - Alphebatically smaller is better.
+      // - Alphabetically smaller is better.
       if (L.Quality.NumberOfParameters != R.Quality.NumberOfParameters)
         return L.Quality.NumberOfParameters < R.Quality.NumberOfParameters;
       if (L.Quality.NumberOfOptionalParameters !=
@@ -1510,7 +1510,7 @@ class CodeCompleteFlow {
   }
 
   // Merges Sema and Index results where possible, to form CompletionCandidates.
-  // \p Identifiers is raw idenfiers that can also be completion condidates.
+  // \p Identifiers is raw idenfiers that can also be completion candidates.
   // Identifiers are not merged with results from index or sema.
   // Groups overloads if desired, to form CompletionCandidate::Bundles. The
   // bundles are scored and top results are returned, best to worst.

clang-tools-extra/clangd/Diagnostics.cpp

@@ -232,7 +232,7 @@ std::string capitalize(std::string Message) {
 }
 
 /// Returns a message sent to LSP for the main diagnostic in \p D.
-/// This message may include notes, if they're not emited in some other way.
+/// This message may include notes, if they're not emitted in some other way.
 /// Example output:
 ///
 ///     no matching function for call to 'foo'

clang-tools-extra/clangd/Diagnostics.h

@@ -53,7 +53,7 @@ struct ClangdDiagnosticOptions {
 struct DiagBase {
   std::string Message;
   // Intended to be used only in error messages.
-  // May be relative, absolute or even artifically constructed.
+  // May be relative, absolute or even artificially constructed.
   std::string File;
   // Absolute path to containing file, if available.
   llvm::Optional<std::string> AbsFile;