Merge from 'main' to 'sycl-web' (#3)

  CONFLICT (content): Merge conflict in clang/lib/Frontend/CompilerInvocation.cpp
  CONFLICT (content): Merge conflict in clang/include/clang/Driver/Options.td

Author: ochupina

clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp

@@ -16,6 +16,77 @@
 using namespace clang::ast_matchers;
 
 namespace clang {
+namespace ast_matchers {
+AST_POLYMORPHIC_MATCHER_P2(hasAnyArgumentWithParam,
+                           AST_POLYMORPHIC_SUPPORTED_TYPES(CallExpr,
+                                                           CXXConstructExpr),
+                           internal::Matcher<Expr>, ArgMatcher,
+                           internal::Matcher<ParmVarDecl>, ParamMatcher) {
+  BoundNodesTreeBuilder Result;
+  // The first argument of an overloaded member operator is the implicit object
+  // argument of the method which should not be matched against a parameter, so
+  // we skip over it here.
+  BoundNodesTreeBuilder Matches;
+  unsigned ArgIndex = cxxOperatorCallExpr(callee(cxxMethodDecl()))
+                              .matches(Node, Finder, &Matches)
+                          ? 1
+                          : 0;
+  int ParamIndex = 0;
+  for (; ArgIndex < Node.getNumArgs(); ++ArgIndex) {
+    BoundNodesTreeBuilder ArgMatches(*Builder);
+    if (ArgMatcher.matches(*(Node.getArg(ArgIndex)->IgnoreParenCasts()), Finder,
+                           &ArgMatches)) {
+      BoundNodesTreeBuilder ParamMatches(ArgMatches);
+      if (expr(anyOf(cxxConstructExpr(hasDeclaration(cxxConstructorDecl(
+                         hasParameter(ParamIndex, ParamMatcher)))),
+                     callExpr(callee(functionDecl(
+                         hasParameter(ParamIndex, ParamMatcher))))))
+              .matches(Node, Finder, &ParamMatches)) {
+        Result.addMatch(ParamMatches);
+        *Builder = std::move(Result);
+        return true;
+      }
+    }
+    ++ParamIndex;
+  }
+  return false;
+}
+
+AST_MATCHER(Expr, usedInBooleanContext) {
+  const char *ExprName = "__booleanContextExpr";
+  auto Result =
+      expr(expr().bind(ExprName),
+           anyOf(hasParent(varDecl(hasType(booleanType()))),
+                 hasParent(cxxConstructorDecl(
+                     hasAnyConstructorInitializer(cxxCtorInitializer(
+                         withInitializer(expr(equalsBoundNode(ExprName))),
+                         forField(hasType(booleanType())))))),
+                 hasParent(fieldDecl(hasType(booleanType()))),
+                 hasParent(stmt(anyOf(
+                     explicitCastExpr(hasDestinationType(booleanType())),
+                     ifStmt(hasCondition(expr(equalsBoundNode(ExprName)))),
+                     doStmt(hasCondition(expr(equalsBoundNode(ExprName)))),
+                     whileStmt(hasCondition(expr(equalsBoundNode(ExprName)))),
+                     forStmt(hasCondition(expr(equalsBoundNode(ExprName)))),
+                     conditionalOperator(
+                         hasCondition(expr(equalsBoundNode(ExprName)))),
+                     parenListExpr(hasParent(varDecl(hasType(booleanType())))),
+                     parenExpr(hasParent(
+                         explicitCastExpr(hasDestinationType(booleanType())))),
+                     returnStmt(forFunction(returns(booleanType()))),
+                     cxxUnresolvedConstructExpr(hasType(booleanType())),
+                     callExpr(hasAnyArgumentWithParam(
+                         expr(equalsBoundNode(ExprName)),
+                         parmVarDecl(hasType(booleanType())))),
+                     binaryOperator(hasAnyOperatorName("&&", "||")),
+                     unaryOperator(hasOperatorName("!")).bind("NegOnSize"))))))
+          .matches(Node, Finder, Builder);
+  Builder->removeBindings([ExprName](const BoundNodesMap &Nodes) {
+    return Nodes.getNode(ExprName).getNodeKind().isNone();
+  });
+  return Result;
+}
+} // namespace ast_matchers
 namespace tidy {
 namespace readability {
 
@@ -26,18 +97,27 @@ ContainerSizeEmptyCheck::ContainerSizeEmptyCheck(StringRef Name,
     : ClangTidyCheck(Name, Context) {}
 
 void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) {
-  const auto ValidContainer = qualType(hasUnqualifiedDesugaredType(
-      recordType(hasDeclaration(cxxRecordDecl(isSameOrDerivedFrom(
-          namedDecl(
-              has(cxxMethodDecl(
-                      isConst(), parameterCountIs(0), isPublic(),
-                      hasName("size"),
-                      returns(qualType(isInteger(), unless(booleanType()))))
-                      .bind("size")),
-              has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
-                                hasName("empty"), returns(booleanType()))
-                      .bind("empty")))
-              .bind("container")))))));
+  const auto ValidContainerRecord = cxxRecordDecl(isSameOrDerivedFrom(
+      namedDecl(
+          has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
+                            hasName("size"),
+                            returns(qualType(isInteger(), unless(booleanType()),
+                                             unless(elaboratedType()))))
+                  .bind("size")),
+          has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
+                            hasName("empty"), returns(booleanType()))
+                  .bind("empty")))
+          .bind("container")));
+
+  const auto ValidContainerNonTemplateType =
+      qualType(hasUnqualifiedDesugaredType(
+          recordType(hasDeclaration(ValidContainerRecord))));
+  const auto ValidContainerTemplateType =
+      qualType(hasUnqualifiedDesugaredType(templateSpecializationType(
+          hasDeclaration(classTemplateDecl(has(ValidContainerRecord))))));
+
+  const auto ValidContainer = qualType(
+      anyOf(ValidContainerNonTemplateType, ValidContainerTemplateType));
 
   const auto WrongUse = traverse(
       TK_AsIs,
@@ -52,18 +132,34 @@ void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) {
               anyOf(hasParent(
                         unaryOperator(hasOperatorName("!")).bind("NegOnSize")),
                     anything()))),
-          hasParent(explicitCastExpr(hasDestinationType(booleanType())))));
+          usedInBooleanContext()));
 
   Finder->addMatcher(
-      cxxMemberCallExpr(on(expr(anyOf(hasType(ValidContainer),
+      cxxMemberCallExpr(unless(isInTemplateInstantiation()),
+                        on(expr(anyOf(hasType(ValidContainer),
                                       hasType(pointsTo(ValidContainer)),
-                                      hasType(references(ValidContainer))))),
+                                      hasType(references(ValidContainer))))
+                               .bind("MemberCallObject")),
                         callee(cxxMethodDecl(hasName("size"))), WrongUse,
                         unless(hasAncestor(cxxMethodDecl(
                             ofClass(equalsBoundNode("container"))))))
           .bind("SizeCallExpr"),
       this);
 
+  Finder->addMatcher(
+      callExpr(has(cxxDependentScopeMemberExpr(
+                   hasObjectExpression(
+                       expr(anyOf(hasType(ValidContainer),
+                                  hasType(pointsTo(ValidContainer)),
+                                  hasType(references(ValidContainer))))
+                           .bind("MemberCallObject")),
+                   hasMemberName("size"))),
+               WrongUse,
+               unless(hasAncestor(
+                   cxxMethodDecl(ofClass(equalsBoundNode("container"))))))
+          .bind("SizeCallExpr"),
+      this);
+
   // Empty constructor matcher.
   const auto DefaultConstructor = cxxConstructExpr(
           hasDeclaration(cxxConstructorDecl(isDefaultConstructor())));
@@ -72,12 +168,11 @@ void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) {
       ignoringImpCasts(stringLiteral(hasSize(0))),
       ignoringImpCasts(cxxBindTemporaryExpr(has(DefaultConstructor))),
       ignoringImplicit(DefaultConstructor),
-      cxxConstructExpr(
-          hasDeclaration(cxxConstructorDecl(isCopyConstructor())),
-          has(expr(ignoringImpCasts(DefaultConstructor)))),
-      cxxConstructExpr(
-          hasDeclaration(cxxConstructorDecl(isMoveConstructor())),
-          has(expr(ignoringImpCasts(DefaultConstructor)))));
+      cxxConstructExpr(hasDeclaration(cxxConstructorDecl(isCopyConstructor())),
+                       has(expr(ignoringImpCasts(DefaultConstructor)))),
+      cxxConstructExpr(hasDeclaration(cxxConstructorDecl(isMoveConstructor())),
+                       has(expr(ignoringImpCasts(DefaultConstructor)))),
+      cxxUnresolvedConstructExpr(argumentCountIs(0)));
   // Match the object being compared.
   const auto STLArg =
       anyOf(unaryOperator(
@@ -87,31 +182,41 @@ void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) {
             expr(hasType(ValidContainer)).bind("STLObject"));
   Finder->addMatcher(
       cxxOperatorCallExpr(
+          unless(isInTemplateInstantiation()),
           hasAnyOverloadedOperatorName("==", "!="),
           anyOf(allOf(hasArgument(0, WrongComparend), hasArgument(1, STLArg)),
                 allOf(hasArgument(0, STLArg), hasArgument(1, WrongComparend))),
           unless(hasAncestor(
               cxxMethodDecl(ofClass(equalsBoundNode("container"))))))
           .bind("BinCmp"),
       this);
+  Finder->addMatcher(
+      binaryOperator(hasAnyOperatorName("==", "!="),
+                     anyOf(allOf(hasLHS(WrongComparend), hasRHS(STLArg)),
+                           allOf(hasLHS(STLArg), hasRHS(WrongComparend))),
+                     unless(hasAncestor(
+                         cxxMethodDecl(ofClass(equalsBoundNode("container"))))))
+          .bind("BinCmp"),
+      this);
 }
 
 void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *MemberCall =
-      Result.Nodes.getNodeAs<CXXMemberCallExpr>("SizeCallExpr");
+  const auto *MemberCall = Result.Nodes.getNodeAs<Expr>("SizeCallExpr");
+  const auto *MemberCallObject =
+      Result.Nodes.getNodeAs<Expr>("MemberCallObject");
   const auto *BinCmp = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("BinCmp");
+  const auto *BinCmpTempl = Result.Nodes.getNodeAs<BinaryOperator>("BinCmp");
   const auto *BinaryOp = Result.Nodes.getNodeAs<BinaryOperator>("SizeBinaryOp");
   const auto *Pointee = Result.Nodes.getNodeAs<Expr>("Pointee");
   const auto *E =
-      MemberCall
-          ? MemberCall->getImplicitObjectArgument()
+      MemberCallObject
+          ? MemberCallObject
           : (Pointee ? Pointee : Result.Nodes.getNodeAs<Expr>("STLObject"));
   FixItHint Hint;
   std::string ReplacementText = std::string(
       Lexer::getSourceText(CharSourceRange::getTokenRange(E->getSourceRange()),
                            *Result.SourceManager, getLangOpts()));
-  if (BinCmp && IsBinaryOrTernary(E)) {
-    // Not just a DeclRefExpr, so parenthesize to be on the safe side.
+  if (IsBinaryOrTernary(E) || isa<UnaryOperator>(E)) {
     ReplacementText = "(" + ReplacementText + ")";
   }
   if (E->getType()->isPointerType())
@@ -125,7 +230,13 @@ void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) {
     }
     Hint =
         FixItHint::CreateReplacement(BinCmp->getSourceRange(), ReplacementText);
-  } else if (BinaryOp) {  // Determine the correct transformation.
+  } else if (BinCmpTempl) {
+    if (BinCmpTempl->getOpcode() == BinaryOperatorKind::BO_NE) {
+      ReplacementText = "!" + ReplacementText;
+    }
+    Hint = FixItHint::CreateReplacement(BinCmpTempl->getSourceRange(),
+                                        ReplacementText);
+  } else if (BinaryOp) { // Determine the correct transformation.
     bool Negation = false;
     const bool ContainerIsLHS =
         !llvm::isa<IntegerLiteral>(BinaryOp->getLHS()->IgnoreImpCasts());
@@ -195,15 +306,17 @@ void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) {
                                           "!" + ReplacementText);
   }
 
-  if (MemberCall) {
-    diag(MemberCall->getBeginLoc(),
-         "the 'empty' method should be used to check "
-         "for emptiness instead of 'size'")
+  auto WarnLoc = MemberCall ? MemberCall->getBeginLoc() : SourceLocation{};
+
+  if (WarnLoc.isValid()) {
+    diag(WarnLoc, "the 'empty' method should be used to check "
+                  "for emptiness instead of 'size'")
         << Hint;
   } else {
-    diag(BinCmp->getBeginLoc(),
-         "the 'empty' method should be used to check "
-         "for emptiness instead of comparing to an empty object")
+    WarnLoc = BinCmpTempl ? BinCmpTempl->getBeginLoc()
+                          : (BinCmp ? BinCmp->getBeginLoc() : SourceLocation{});
+    diag(WarnLoc, "the 'empty' method should be used to check "
+                  "for emptiness instead of comparing to an empty object")
         << Hint;
   }
 

clang-tools-extra/clangd/CMakeLists.txt

@@ -14,9 +14,12 @@ if (NOT DEFINED CLANGD_BUILD_XPC)
   unset(CLANGD_BUILD_XPC_DEFAULT)
 endif ()
 
+option(CLANGD_MALLOC_TRIM "Call malloc_trim(3) periodically in Clangd. (only takes effect when using glibc)" ON)
+
 llvm_canonicalize_cmake_booleans(
   CLANGD_BUILD_XPC
   CLANGD_ENABLE_REMOTE
+  CLANGD_MALLOC_TRIM
   LLVM_ENABLE_ZLIB
 )
 

clang-tools-extra/clangd/ClangdLSPServer.cpp

@@ -178,6 +178,7 @@ class ClangdLSPServer::MessageHandler : public Transport::MessageHandler {
     } else if (auto Handler = Notifications.lookup(Method)) {
       Handler(std::move(Params));
       Server.maybeExportMemoryProfile();
+      Server.maybeCleanupMemory();
     } else {
       log("unhandled notification {0}", Method);
     }
@@ -453,6 +454,7 @@ void ClangdLSPServer::callRaw(StringRef Method, llvm::json::Value Params,
 
 void ClangdLSPServer::notify(llvm::StringRef Method, llvm::json::Value Params) {
   log("--> {0}", Method);
+  maybeCleanupMemory();
   std::lock_guard<std::mutex> Lock(TranspWriter);
   Transp.notify(Method, std::move(Params));
 }
@@ -1283,13 +1285,7 @@ void ClangdLSPServer::publishDiagnostics(
 }
 
 void ClangdLSPServer::maybeExportMemoryProfile() {
-  if (!trace::enabled())
-    return;
-  // Profiling might be expensive, so we throttle it to happen once every 5
-  // minutes.
-  static constexpr auto ProfileInterval = std::chrono::minutes(5);
-  auto Now = std::chrono::steady_clock::now();
-  if (Now < NextProfileTime)
+  if (!trace::enabled() || !ShouldProfile())
     return;
 
   static constexpr trace::Metric MemoryUsage(
@@ -1298,7 +1294,12 @@ void ClangdLSPServer::maybeExportMemoryProfile() {
   MemoryTree MT;
   profile(MT);
   record(MT, "clangd_lsp_server", MemoryUsage);
-  NextProfileTime = Now + ProfileInterval;
+}
+
+void ClangdLSPServer::maybeCleanupMemory() {
+  if (!Opts.MemoryCleanup || !ShouldCleanupMemory())
+    return;
+  Opts.MemoryCleanup();
 }
 
 // FIXME: This function needs to be properly tested.
@@ -1458,10 +1459,15 @@ void ClangdLSPServer::onAST(const ASTParams &Params,
 ClangdLSPServer::ClangdLSPServer(class Transport &Transp,
                                  const ThreadsafeFS &TFS,
                                  const ClangdLSPServer::Options &Opts)
-    : BackgroundContext(Context::current().clone()), Transp(Transp),
+    : ShouldProfile(/*Period=*/std::chrono::minutes(5),
+                    /*Delay=*/std::chrono::minutes(1)),
+      ShouldCleanupMemory(/*Period=*/std::chrono::minutes(1),
+                          /*Delay=*/std::chrono::minutes(1)),
+      BackgroundContext(Context::current().clone()), Transp(Transp),
       MsgHandler(new MessageHandler(*this)), TFS(TFS),
       SupportedSymbolKinds(defaultSymbolKinds()),
       SupportedCompletionItemKinds(defaultCompletionItemKinds()), Opts(Opts) {
+
   // clang-format off
   MsgHandler->bind("initialize", &ClangdLSPServer::onInitialize);
   MsgHandler->bind("initialized", &ClangdLSPServer::onInitialized);
@@ -1506,9 +1512,6 @@ ClangdLSPServer::ClangdLSPServer(class Transport &Transp,
   if (Opts.FoldingRanges)
     MsgHandler->bind("textDocument/foldingRange", &ClangdLSPServer::onFoldingRange);
   // clang-format on
-
-  // Delay first profile until we've finished warming up.
-  NextProfileTime = std::chrono::steady_clock::now() + std::chrono::minutes(1);
 }
 
 ClangdLSPServer::~ClangdLSPServer() {
@@ -1621,6 +1624,10 @@ void ClangdLSPServer::onDiagnosticsReady(PathRef File, llvm::StringRef Version,
 void ClangdLSPServer::onBackgroundIndexProgress(
     const BackgroundQueue::Stats &Stats) {
   static const char ProgressToken[] = "backgroundIndexProgress";
+
+  // The background index did some work, maybe we need to cleanup
+  maybeCleanupMemory();
+
   std::lock_guard<std::mutex> Lock(BackgroundIndexProgressMutex);
 
   auto NotifyProgress = [this](const BackgroundQueue::Stats &Stats) {

clang-tools-extra/clangd/ClangdLSPServer.h

@@ -19,6 +19,7 @@
 #include "support/Context.h"
 #include "support/MemoryTree.h"
 #include "support/Path.h"
+#include "support/Threading.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringSet.h"
@@ -48,6 +49,9 @@ class ClangdLSPServer : private ClangdServer::Callbacks {
     llvm::Optional<Path> CompileCommandsDir;
     /// The offset-encoding to use, or None to negotiate it over LSP.
     llvm::Optional<OffsetEncoding> Encoding;
+    /// If set, periodically called to release memory.
+    /// Consider malloc_trim(3)
+    std::function<void()> MemoryCleanup = nullptr;
 
     /// Per-feature options. Generally ClangdServer lets these vary
     /// per-request, but LSP allows limited/no customizations.
@@ -183,10 +187,12 @@ class ClangdLSPServer : private ClangdServer::Callbacks {
   /// Runs profiling and exports memory usage metrics if tracing is enabled and
   /// profiling hasn't happened recently.
   void maybeExportMemoryProfile();
+  PeriodicThrottler ShouldProfile;
 
-  /// Timepoint until which profiling is off. It is used to throttle profiling
-  /// requests.
-  std::chrono::steady_clock::time_point NextProfileTime;
+  /// Run the MemoryCleanup callback if it's time.
+  /// This method is thread safe.
+  void maybeCleanupMemory();
+  PeriodicThrottler ShouldCleanupMemory;
 
   /// Since initialization of CDBs and ClangdServer is done lazily, the
   /// following context captures the one used while creating ClangdLSPServer and