LLVM and SPIRV-LLVM-Translator pulldown (WW17) #3573

LLVM: llvm/llvm-project@0a92aff
SPIRV-LLVM-Translator: KhronosGroup/SPIRV-LLVM-Translator@b9e4fa4

Author: vladimirlaz

clang-tools-extra/clang-tidy/ClangTidy.cpp

@@ -132,6 +132,8 @@ class ErrorReporter {
       }
       auto Diag = Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0 [%1]"))
                   << Message.Message << Name;
+      for (const FileByteRange &FBR : Error.Message.Ranges)
+        Diag << getRange(FBR);
       // FIXME: explore options to support interactive fix selection.
       const llvm::StringMap<Replacements> *ChosenFix;
       if (ApplyFixes != FB_NoFix &&
@@ -257,17 +259,13 @@ class ErrorReporter {
       for (const auto &Repl : FileAndReplacements.second) {
         if (!Repl.isApplicable())
           continue;
-        SmallString<128> FixAbsoluteFilePath = Repl.getFilePath();
-        Files.makeAbsolutePath(FixAbsoluteFilePath);
-        SourceLocation FixLoc =
-            getLocation(FixAbsoluteFilePath, Repl.getOffset());
-        SourceLocation FixEndLoc = FixLoc.getLocWithOffset(Repl.getLength());
-        // Retrieve the source range for applicable fixes. Macro definitions
-        // on the command line have locations in a virtual buffer and don't
-        // have valid file paths and are therefore not applicable.
-        CharSourceRange Range =
-            CharSourceRange::getCharRange(SourceRange(FixLoc, FixEndLoc));
-        Diag << FixItHint::CreateReplacement(Range, Repl.getReplacementText());
+        FileByteRange FBR;
+        FBR.FilePath = Repl.getFilePath().str();
+        FBR.FileOffset = Repl.getOffset();
+        FBR.Length = Repl.getLength();
+
+        Diag << FixItHint::CreateReplacement(getRange(FBR),
+                                             Repl.getReplacementText());
       }
     }
   }
@@ -277,9 +275,22 @@ class ErrorReporter {
     auto Diag =
         Diags.Report(Loc, Diags.getCustomDiagID(DiagnosticsEngine::Note, "%0"))
         << Message.Message;
+    for (const FileByteRange &FBR : Message.Ranges)
+      Diag << getRange(FBR);
     reportFix(Diag, Message.Fix);
   }
 
+  CharSourceRange getRange(const FileByteRange &Range) {
+    SmallString<128> AbsoluteFilePath{Range.FilePath};
+    Files.makeAbsolutePath(AbsoluteFilePath);
+    SourceLocation BeginLoc = getLocation(AbsoluteFilePath, Range.FileOffset);
+    SourceLocation EndLoc = BeginLoc.getLocWithOffset(Range.Length);
+    // Retrieve the source range for applicable highlights and fixes. Macro
+    // definition on the command line have locations in a virtual buffer and
+    // don't have valid file paths and are therefore not applicable.
+    return CharSourceRange::getCharRange(BeginLoc, EndLoc);
+  }
+
   FileManager Files;
   LangOptions LangOpts; // FIXME: use langopts from each original file
   IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts;

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp

@@ -25,6 +25,7 @@
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/DiagnosticRenderer.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Core/Diagnostic.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/STLExtras.h"
@@ -62,15 +63,32 @@ class ClangTidyDiagnosticRenderer : public DiagnosticRenderer {
         Loc.isValid()
             ? tooling::DiagnosticMessage(Message, Loc.getManager(), Loc)
             : tooling::DiagnosticMessage(Message);
+
+    // Make sure that if a TokenRange is receieved from the check it is unfurled
+    // into a real CharRange for the diagnostic printer later.
+    // Whatever we store here gets decoupled from the current SourceManager, so
+    // we **have to** know the exact position and length of the highlight.
+    auto ToCharRange = [this, &Loc](const CharSourceRange &SourceRange) {
+      if (SourceRange.isCharRange())
+        return SourceRange;
+      assert(SourceRange.isTokenRange());
+      SourceLocation End = Lexer::getLocForEndOfToken(
+          SourceRange.getEnd(), 0, Loc.getManager(), LangOpts);
+      return CharSourceRange::getCharRange(SourceRange.getBegin(), End);
+    };
+
     if (Level == DiagnosticsEngine::Note) {
       Error.Notes.push_back(TidyMessage);
+      for (const CharSourceRange &SourceRange : Ranges)
+        Error.Notes.back().Ranges.emplace_back(Loc.getManager(),
+                                               ToCharRange(SourceRange));
       return;
     }
     assert(Error.Message.Message.empty() && "Overwriting a diagnostic message");
     Error.Message = TidyMessage;
-    for (const CharSourceRange &SourceRange : Ranges) {
-      Error.Ranges.emplace_back(Loc.getManager(), SourceRange);
-    }
+    for (const CharSourceRange &SourceRange : Ranges)
+      Error.Message.Ranges.emplace_back(Loc.getManager(),
+                                        ToCharRange(SourceRange));
   }
 
   void emitDiagnosticLoc(FullSourceLoc Loc, PresumedLoc PLoc,

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

@@ -213,6 +213,9 @@ class FunctionASTVisitor final
     : public RecursiveASTVisitor<FunctionASTVisitor> {
   using Base = RecursiveASTVisitor<FunctionASTVisitor>;
 
+  // If set to true, macros are ignored during analysis.
+  const bool IgnoreMacros;
+
   // The current nesting level (increased by Criteria::IncrementNesting).
   unsigned short CurrentNestingLevel = 0;
 
@@ -223,6 +226,9 @@ class FunctionASTVisitor final
   std::stack<OBO, SmallVector<OBO, 4>> BinaryOperatorsStack;
 
 public:
+  explicit FunctionASTVisitor(const bool IgnoreMacros)
+      : IgnoreMacros(IgnoreMacros) {}
+
   bool traverseStmtWithIncreasedNestingLevel(Stmt *Node) {
     ++CurrentNestingLevel;
     bool ShouldContinue = Base::TraverseStmt(Node);
@@ -364,6 +370,9 @@ class FunctionASTVisitor final
     if (!Node)
       return Base::TraverseStmt(Node);
 
+    if (IgnoreMacros && Node->getBeginLoc().isMacroID())
+      return true;
+
     // Three following switch()'es have huge duplication, but it is better to
     // keep them separate, to simplify comparing them with the Specification.
 
@@ -493,12 +502,14 @@ FunctionCognitiveComplexityCheck::FunctionCognitiveComplexityCheck(
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       Threshold(Options.get("Threshold", CognitiveComplexity::DefaultLimit)),
-      DescribeBasicIncrements(Options.get("DescribeBasicIncrements", true)) {}
+      DescribeBasicIncrements(Options.get("DescribeBasicIncrements", true)),
+      IgnoreMacros(Options.get("IgnoreMacros", false)) {}
 
 void FunctionCognitiveComplexityCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "Threshold", Threshold);
   Options.store(Opts, "DescribeBasicIncrements", DescribeBasicIncrements);
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
 }
 
 void FunctionCognitiveComplexityCheck::registerMatchers(MatchFinder *Finder) {
@@ -513,7 +524,7 @@ void FunctionCognitiveComplexityCheck::registerMatchers(MatchFinder *Finder) {
 void FunctionCognitiveComplexityCheck::check(
     const MatchFinder::MatchResult &Result) {
 
-  FunctionASTVisitor Visitor;
+  FunctionASTVisitor Visitor(IgnoreMacros);
   SourceLocation Loc;
 
   const auto *TheDecl = Result.Nodes.getNodeAs<FunctionDecl>("func");

clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h

@@ -26,6 +26,8 @@ namespace readability {
 ///     diagnostics on every piece of code (loop, `if` statement, etc.) which
 ///     contributes to that complexity.
 //      Default is `true`
+///   * `IgnoreMacros` - if set to `true`, the check will ignore code inside
+///     macros. Default is `false`.
 ///
 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/readability-function-cognitive-complexity.html
@@ -43,6 +45,7 @@ class FunctionCognitiveComplexityCheck : public ClangTidyCheck {
 private:
   const unsigned Threshold;
   const bool DescribeBasicIncrements;
+  const bool IgnoreMacros;
 };
 
 } // namespace readability

clang-tools-extra/clangd/Config.h

@@ -70,7 +70,7 @@ struct Config {
   enum class BackgroundPolicy { Build, Skip };
   /// Describes an external index configuration.
   struct ExternalIndexSpec {
-    enum { File, Server } Kind;
+    enum { None, File, Server } Kind;
     /// This is one of:
     /// - Address of a clangd-index-server, in the form of "ip:port".
     /// - Absolute path to an index produced by clangd-indexer.

clang-tools-extra/clangd/ConfigCompile.cpp

@@ -322,16 +322,18 @@ struct FragmentCompiler {
                llvm::SMRange BlockRange) {
 #ifndef CLANGD_ENABLE_REMOTE
     if (External.Server) {
-      diag(Error, "Clangd isn't compiled with remote index support, ignoring "
-                  "Server." External.Server->Range);
+      elog("Clangd isn't compiled with remote index support, ignoring Server: "
+           "{0}",
+           *External.Server);
       External.Server.reset();
     }
 #endif
     // Make sure exactly one of the Sources is set.
-    unsigned SourceCount =
-        External.File.hasValue() + External.Server.hasValue();
+    unsigned SourceCount = External.File.hasValue() +
+                           External.Server.hasValue() + *External.IsNone;
     if (SourceCount != 1) {
-      diag(Error, "Exactly one of File or Server must be set.", BlockRange);
+      diag(Error, "Exactly one of File, Server or None must be set.",
+           BlockRange);
       return;
     }
     Config::ExternalIndexSpec Spec;
@@ -345,20 +347,29 @@ struct FragmentCompiler {
       if (!AbsPath)
         return;
       Spec.Location = std::move(*AbsPath);
+    } else {
+      assert(*External.IsNone);
+      Spec.Kind = Config::ExternalIndexSpec::None;
     }
-    // Make sure MountPoint is an absolute path with forward slashes.
-    if (!External.MountPoint)
-      External.MountPoint.emplace(FragmentDirectory);
-    if ((**External.MountPoint).empty()) {
-      diag(Error, "A mountpoint is required.", BlockRange);
-      return;
+    if (Spec.Kind != Config::ExternalIndexSpec::None) {
+      // Make sure MountPoint is an absolute path with forward slashes.
+      if (!External.MountPoint)
+        External.MountPoint.emplace(FragmentDirectory);
+      if ((**External.MountPoint).empty()) {
+        diag(Error, "A mountpoint is required.", BlockRange);
+        return;
+      }
+      auto AbsPath = makeAbsolute(std::move(*External.MountPoint), "MountPoint",
+                                  llvm::sys::path::Style::posix);
+      if (!AbsPath)
+        return;
+      Spec.MountPoint = std::move(*AbsPath);
     }
-    auto AbsPath = makeAbsolute(std::move(*External.MountPoint), "MountPoint",
-                                llvm::sys::path::Style::posix);
-    if (!AbsPath)
-      return;
-    Spec.MountPoint = std::move(*AbsPath);
     Out.Apply.push_back([Spec(std::move(Spec))](const Params &P, Config &C) {
+      if (Spec.Kind == Config::ExternalIndexSpec::None) {
+        C.Index.External.reset();
+        return;
+      }
       if (P.Path.empty() || !pathStartsWith(Spec.MountPoint, P.Path,
                                             llvm::sys::path::Style::posix))
         return;

clang-tools-extra/clangd/ConfigFragment.h

@@ -173,6 +173,9 @@ struct Fragment {
     /// usually prepared using clangd-indexer.
     /// Exactly one source (File/Server) should be configured.
     struct ExternalBlock {
+      /// Whether the block is explicitly set to `None`. Can be used to clear
+      /// any external index specified before.
+      Located<bool> IsNone = false;
       /// Path to an index file generated by clangd-indexer. Relative paths may
       /// be used, if config fragment is associated with a directory.
       llvm::Optional<Located<std::string>> File;

clang-tools-extra/clangd/ConfigYAML.cpp

@@ -12,6 +12,7 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/YAMLParser.h"
+#include <string>
 #include <system_error>
 
 namespace clang {
@@ -149,13 +150,34 @@ class Parser {
                 [&](Node &N) { F.Background = scalarValue(N, "Background"); });
     Dict.handle("External", [&](Node &N) {
       Fragment::IndexBlock::ExternalBlock External;
-      parse(External, N);
+      // External block can either be a mapping or a scalar value. Dispatch
+      // accordingly.
+      if (N.getType() == Node::NK_Mapping) {
+        parse(External, N);
+      } else if (N.getType() == Node::NK_Scalar ||
+                 N.getType() == Node::NK_BlockScalar) {
+        parse(External, scalarValue(N, "External").getValue());
+      } else {
+        error("External must be either a scalar or a mapping.", N);
+        return;
+      }
       F.External.emplace(std::move(External));
       F.External->Range = N.getSourceRange();
     });
     Dict.parse(N);
   }
 
+  void parse(Fragment::IndexBlock::ExternalBlock &F,
+             Located<std::string> ExternalVal) {
+    if (!llvm::StringRef(*ExternalVal).equals_lower("none")) {
+      error("Only scalar value supported for External is 'None'",
+            ExternalVal.Range);
+      return;
+    }
+    F.IsNone = true;
+    F.IsNone.Range = ExternalVal.Range;
+  }
+
   void parse(Fragment::IndexBlock::ExternalBlock &F, Node &N) {
     DictParser Dict("External", this);
     Dict.handle("File", [&](Node &N) { F.File = scalarValue(N, "File"); });

clang-tools-extra/clangd/test/check-lines.test

@@ -0,0 +1,15 @@
+// RUN: cp %s %t.cpp
+// RUN: not clangd -check=%t.cpp -check-lines=6-14 2>&1 | FileCheck -strict-whitespace %s
+// RUN: not clangd -check=%t.cpp -check-lines=14 2>&1 | FileCheck -strict-whitespace %s
+
+// CHECK: Testing on source file {{.*}}check-lines.test
+// CHECK: internal (cc1) args are: -cc1
+// CHECK: Building preamble...
+// CHECK: Building AST...
+// CHECK: Testing features at each token
+// CHECK: tweak: ExpandAutoType ==> FAIL
+// CHECK: All checks completed, 1 errors
+
+void fun();
+auto x = fun; // This line is tested
+auto y = fun; // This line is not tested

clang-tools-extra/clangd/tool/Check.cpp

@@ -192,14 +192,19 @@ class Checker {
   }
 
   // Run AST-based features at each token in the file.
-  void testLocationFeatures() {
+  void testLocationFeatures(
+      llvm::function_ref<bool(const Position &)> ShouldCheckLine) {
     log("Testing features at each token (may be slow in large files)");
-    auto SpelledTokens =
-        AST->getTokens().spelledTokens(AST->getSourceManager().getMainFileID());
+    auto &SM = AST->getSourceManager();
+    auto SpelledTokens = AST->getTokens().spelledTokens(SM.getMainFileID());
     for (const auto &Tok : SpelledTokens) {
       unsigned Start = AST->getSourceManager().getFileOffset(Tok.location());
       unsigned End = Start + Tok.length();
       Position Pos = offsetToPosition(Inputs.Contents, Start);
+
+      if (!ShouldCheckLine(Pos))
+        continue;
+
       // FIXME: dumping the tokens may leak sensitive code into bug reports.
       // Add an option to turn this off, once we decide how options work.
       vlog("  {0} {1}", Pos, Tok.text(AST->getSourceManager()));
@@ -229,8 +234,9 @@ class Checker {
 
 } // namespace
 
-bool check(llvm::StringRef File, const ThreadsafeFS &TFS,
-           const ClangdLSPServer::Options &Opts) {
+bool check(llvm::StringRef File,
+           llvm::function_ref<bool(const Position &)> ShouldCheckLine,
+           const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts) {
   llvm::SmallString<0> FakeFile;
   llvm::Optional<std::string> Contents;
   if (File.empty()) {
@@ -254,7 +260,7 @@ bool check(llvm::StringRef File, const ThreadsafeFS &TFS,
   if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents) ||
       !C.buildAST())
     return false;
-  C.testLocationFeatures();
+  C.testLocationFeatures(ShouldCheckLine);
 
   log("All checks completed, {0} errors", C.ErrCount);
   return C.ErrCount == 0;