[clangd] A code tweak to expand a macro

[ilya-biryukov]

This actually works, but still far from landing.
Notable problems:

• this adds a dependency on TokenBuffer, so we need to land it first.
• this change is too big, planning to split into multiple changes: (1) collecting tokens when building the AST for clangd, (2) add helpers into TokenBuffer
• it currently tries to replace any "mapping", not just preprocessor expansions. E.g. will attempt to replace #define FOO ... with an empty string.
• needs more tests.

Will get back to it after TokenBuffer lands.

[ilya-biryukov]

• Move logically separate parts to other changes

[ilya-biryukov]

• Rebase
• Update some comments

[sammccall]

clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp

57 ↗

(On Diff #206388)

it's pretty weird for a function whose return type is spelled "Token*" to use Spelled.end() rather than nullptr as a sentinel

125 ↗

(On Diff #206388)

include macro name?

clang-tools-extra/clangd/unittests/TweakTests.cpp

288 ↗

(On Diff #206388)

Can you verify we don't trigger here? FOO[[ ]]BAR

The zero-width range in FOO^ BAR is indeed interpreted as pointing at FOO by SelectionTree, but that's a whitespace-sensitive heuristic.

Given

int x(int);
#define B x
int y = B^(42);

The ^ points at the (. (Maybe we should lift this logic into Tweak::Selection)

[ilya-biryukov]

• Replace bsearch with partition_point.
• Include macro name in the title.
• Added a FIXME for empty selection case.
• Return null when no token is found.

clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp

57 ↗

(On Diff #206388)

Yeah, especially given that the function below uses nullptr as a sentinel...
Thanks for pointing this out!

clang-tools-extra/clangd/unittests/TweakTests.cpp

288 ↗

(On Diff #206388)

It does trigger, unfortunately. There is no way to unbreak this, as we don't have a way to get the selection range in inputs of the tweak.
I've added a FIXME, will address with a follow-up.

Note that we don't use selection tree, as it's AST-based and we need token-level information for that tweak (that's pretty unique, I expect most tweaks are not like that)

cfe/trunk/include/clang/Tooling/Syntax/Tokens.h

struct FileRange {
   FileID file() const { return File; }
   /// Start is a start offset (inclusive) in the corresponding file.
   unsigned beginOffset() const { return Begin; }
   /// End offset (exclusive) in the corresponding file.
   unsigned endOffset() const { return End; }
 
   unsigned length() const { return End - Begin; }
 
+  /// Check if \p Offset is inside the range.
+  bool contains(unsigned Offset) const {
+    return Begin <= Offset && Offset < End;
+  }
+  /// Check \p Offset is inside the range or equal to its endpoint.
+  bool touches(unsigned Offset) const {
+    return Begin <= Offset && Offset <= End;
+  }
+
   /// Gets the substring that this FileRange refers to.
   llvm::StringRef text(const SourceManager &SM) const;
 
   friend bool operator==(const FileRange &L, const FileRange &R) {
     return std::tie(L.File, L.Begin, L.End) == std::tie(R.File, R.Begin, R.End);
   }
   friend bool operator!=(const FileRange &L, const FileRange &R) {
     return !(L == R);

clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt

 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../..)
 
 set(LLVM_LINK_COMPONENTS
   support
   )
 
 # A target containing all code tweaks (i.e. mini-refactorings) provided by
 # clangd.
 # Built as an object library to make sure the linker does not remove global
 # constructors that register individual tweaks in a global registry.
 # To enable these tweaks in executables or shared libraries, add
 # $<TARGET_OBJECTS:obj.clangDaemonTweaks> to a list of sources, see
 # clangd/tool/CMakeLists.txt for an example.
 add_clang_library(clangDaemonTweaks OBJECT
   AnnotateHighlightings.cpp
   DumpAST.cpp
+  ExpandMacro.cpp
   RawStringLiteral.cpp
   SwapIfBranches.cpp
 
   LINK_LIBS
   clangAST
   clangBasic
   clangDaemon
   clangToolingCore
+  clangToolingSyntax
   )

clang-tools-extra/trunk/clangd/refactor/tweaks/ExpandMacro.cpp

+//===--- ExpandMacro.cpp -----------------------------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "refactor/Tweak.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Syntax/Tokens.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Error.h"
+#include <string>
+namespace clang {
+namespace clangd {
+namespace {
+
+/// Replaces a reference to a macro under the cursor with its expansion.
+/// Before:
+///   #define FOO(X) X+X
+///   FOO(10*a)
+///   ^^^
+/// After:
+///   #define FOO(X) X+X
+///   10*a+10*a
+class ExpandMacro : public Tweak {
+public:
+  const char *id() const override final;
+  Intent intent() const override { return Intent::Refactor; }
+
+  bool prepare(const Selection &Inputs) override;
+  Expected<Tweak::Effect> apply(const Selection &Inputs) override;
+  std::string title() const override;
+
+private:
+  syntax::TokenBuffer::Expansion Expansion;
+  std::string MacroName;
+};
+
+REGISTER_TWEAK(ExpandMacro)
+
+/// Finds a spelled token that the cursor is pointing at.
+static const syntax::Token *
+findTokenUnderCursor(const SourceManager &SM,
+                     llvm::ArrayRef<syntax::Token> Spelled,
+                     unsigned CursorOffset) {
+  // Find the token that strats after the offset, then look at a previous one.
+  auto It = llvm::partition_point(Spelled, [&](const syntax::Token &T) {
+    assert(T.location().isFileID());
+    return SM.getFileOffset(T.location()) <= CursorOffset;
+  });
+  if (It == Spelled.begin())
+    return nullptr;
+  // Check the token we found actually touches the cursor position.
+  --It;
+  return It->range(SM).touches(CursorOffset) ? It : nullptr;
+}
+
+static const syntax::Token *
+findIdentifierUnderCursor(const syntax::TokenBuffer &Tokens,
+                          SourceLocation Cursor) {
+  assert(Cursor.isFileID());
+
+  auto &SM = Tokens.sourceManager();
+  auto Spelled = Tokens.spelledTokens(SM.getFileID(Cursor));
+
+  auto *T = findTokenUnderCursor(SM, Spelled, SM.getFileOffset(Cursor));
+  if (!T)
+    return nullptr;
+  if (T->kind() == tok::identifier)
+    return T;
+  // Also try the previous token when the cursor is at the boundary, e.g.
+  //   FOO^()
+  //   FOO^+
+  if (T == Spelled.begin())
+    return nullptr;
+  --T;
+  if (T->endLocation() != Cursor || T->kind() != tok::identifier)
+    return nullptr;
+  return T;
+}
+
+bool ExpandMacro::prepare(const Selection &Inputs) {
+  // FIXME: we currently succeed on selection at the end of the token, e.g.
+  //        'FOO[[ ]]BAR'. We should not trigger in that case.
+
+  // Find a token under the cursor.
+  auto *T = findIdentifierUnderCursor(Inputs.AST.getTokens(), Inputs.Cursor);
+  // We are interested only in identifiers, other tokens can't be macro names.
+  if (!T)
+    return false;
+  // If the identifier is a macro we will find the corresponding expansion.
+  auto Expansion = Inputs.AST.getTokens().expansionStartingAt(T);
+  if (!Expansion)
+    return false;
+  this->MacroName = T->text(Inputs.AST.getSourceManager());
+  this->Expansion = *Expansion;
+  return true;
+}
+
+Expected<Tweak::Effect> ExpandMacro::apply(const Selection &Inputs) {
+  auto &SM = Inputs.AST.getASTContext().getSourceManager();
+
+  std::string Replacement;
+  for (const syntax::Token &T : Expansion.Expanded) {
+    Replacement += T.text(SM);
+    Replacement += " ";
+  }
+  if (!Replacement.empty()) {
+    assert(Replacement.back() == ' ');
+    Replacement.pop_back();
+  }
+
+  CharSourceRange MacroRange =
+      CharSourceRange::getCharRange(Expansion.Spelled.front().location(),
+                                    Expansion.Spelled.back().endLocation());
+
+  Tweak::Effect E;
+  E.ApplyEdit.emplace();
+  llvm::cantFail(
+      E.ApplyEdit->add(tooling::Replacement(SM, MacroRange, Replacement)));
+  return E;
+}
+
+std::string ExpandMacro::title() const {
+  return llvm::formatv("Expand macro '{0}'", MacroName);
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang

clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp

 TEST(TweakTest, AnnotateHighlightings) {
   llvm::StringLiteral ID = "AnnotateHighlightings";
   checkAvailable(ID, "^vo^id^ ^f(^) {^}^"); // available everywhere.
   const char *Input = "void ^f() {}";
   const char *Output = "void /* entity.name.function.cpp */f() {}";
   checkTransform(ID, Input, Output);
 }
 
+TEST(TweakTest, ExpandMacro) {
+  llvm::StringLiteral ID = "ExpandMacro";
+
+  // Available on macro names, not available anywhere else.
+  checkAvailable(ID, R"cpp(
+#define FOO 1 2 3
+#define FUNC(X) X+X+X
+^F^O^O^ BAR ^F^O^O^
+^F^U^N^C^(1)
+)cpp");
+  checkNotAvailable(ID, R"cpp(
+^#^d^efine^ ^FO^O 1 ^2 ^3^
+FOO ^B^A^R^ FOO ^
+FUNC(^1^)^
+)cpp");
+
+  // Works as expected on object-like macros.
+  checkTransform(ID, R"cpp(
+#define FOO 1 2 3
+^FOO BAR FOO
+)cpp",
+                 R"cpp(
+#define FOO 1 2 3
+1 2 3 BAR FOO
+)cpp");
+  checkTransform(ID, R"cpp(
+#define FOO 1 2 3
+FOO BAR ^FOO
+)cpp",
+                 R"cpp(
+#define FOO 1 2 3
+FOO BAR 1 2 3
+)cpp");
+
+  // And function-like macros.
+  checkTransform(ID, R"cpp(
+#define FUNC(X) X+X+X
+F^UNC(2)
+)cpp",
+                 R"cpp(
+#define FUNC(X) X+X+X
+2 + 2 + 2
+)cpp");
+
+  // Works on empty macros.
+  checkTransform(ID, R"cpp(
+#define EMPTY
+int a ^EMPTY;
+  )cpp",
+                 R"cpp(
+#define EMPTY
+int a ;
+  )cpp");
+  checkTransform(ID, R"cpp(
+#define EMPTY_FN(X)
+int a ^EMPTY_FN(1 2 3);
+  )cpp",
+                 R"cpp(
+#define EMPTY_FN(X)
+int a ;
+  )cpp");
+  checkTransform(ID, R"cpp(
+#define EMPTY
+#define EMPTY_FN(X)
+int a = 123 ^EMPTY EMPTY_FN(1);
+  )cpp",
+                 R"cpp(
+#define EMPTY
+#define EMPTY_FN(X)
+int a = 123  EMPTY_FN(1);
+  )cpp");
+  checkTransform(ID, R"cpp(
+#define EMPTY
+#define EMPTY_FN(X)
+int a = 123 ^EMPTY_FN(1) EMPTY;
+  )cpp",
+                 R"cpp(
+#define EMPTY
+#define EMPTY_FN(X)
+int a = 123  EMPTY;
+  )cpp");
+  checkTransform(ID, R"cpp(
+#define EMPTY
+#define EMPTY_FN(X)
+int a = 123 EMPTY_FN(1) ^EMPTY;
+  )cpp",
+                 R"cpp(
+#define EMPTY
+#define EMPTY_FN(X)
+int a = 123 EMPTY_FN(1) ;
+  )cpp");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang