[clangd] DefineInline action availability checks

[kadircet]

Summary

Introduces DefineInline action and initial version of availability
checks.

[kadircet]

• Rebase

[sammccall]

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

52

This function, and all the other helpers in this file, are actually imminently going away in favor of TweakTesting.h.
Sorry it's been stuck in my backlog for a couple of weeks...

We should work out how to do that stuff there.
One approach is to give TweakTest a StringMap<string> ExtraFiles and StringMap<string> EditedFiles that excludes main, so you'd write

ExtraFiles["foo.h"] = "...";
EXPECT_EQ(apply("..."), "...");
EXPECT_THAT(EditedFiles, ElementsAre(Pair(testPath("foo.h"), "...");

or something like this.

(Whatever we do, we should bear in mind the fact that out-line will mean we need to supply an index too)

894

I don't think so

916

nit: NoForwardDecl

946

FWIW, I find these params rather cryptic - it'd be nice to have an API for the helpers that is explicit about this being files/filenames

[kadircet]

• Rebase and address comments

[arphaman]

When fully implemented, will define inline tweak work with C++ methods in classes as well?
E.g.

HEADER:
class Foo { void foo(); }
CPP:
#include "Header.h"
void Foo::foo() {}

becomes:

HEADER:
class Foo { void foo() { } }
CPP:
#include "Header.h"

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

193

Sorry, I'm not really familiar with design of tweaks, so this might be a bad question: Is it possible to change the title of the tweak on a per-TU basis. More specifically, some tweaks might want to have different titles for Objective-C actions compared to their C++ siblings.

[kadircet]

In D65433#1661865, @arphaman wrote:

When fully implemented, will define inline tweak work with C++ methods in classes as well?

Yes you can see an example of it in the tests provided in TweakTests.cpp

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

193

Current design returns a constant string per tweak, but we can change that behaviour easily by making this a function of language options. You can take a look at ClangdServer::enumerateTweaks for details.

[sammccall]

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

193

This isn't quite true: id() must be constant but title() need not be. See ExpandMacro for an example.

The only constraint is it shouldn't be terribly expensive (this is why ExpandAuto doesn't include the type in the title, due to an AST bug it requires a new traversal).

More specifically, some tweaks might want to have different titles for Objective-C actions compared to their C++ siblings.

I think this is possible and the right idea. But for completeness I should mention: having multiple Tweak subclasses that share implementation is also possible. In particular in cases where you have two related actions and may want to provide *both* as options (maybe extract function vs method?), they'd need to be separate subclasses. (Or we'd need to change the design a bit)

[arphaman]

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

193

Oh yes, I think it's probably better to use the Tweak subclasses like you said. This will be better for ObjC++, which could have either a C++ or an ObjC tweak available depending on the current AST selection.

[hokein]

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

66

nit: maybe use auto?

79

thinking this a bit more, the function is non-trivial, we probably want unit test for it.

106

IIUC, I believe this function relies on the flag "Opts.IndexFunctionLocals"? could we spell it explicitly in the code even its default value false fits our use case.

114

hmm, looks like we don't use TargetContext in the implemenation at all. Is the comment out-of-date now?

142

we are distinguishing two cases here: normal functions and the class methods. Could we make the code handle these two cases separately? e.g.

if (Class) {
   // check whether they are in the same class;
} else { // handle normal function
  // call isBeforeInTranslation
}

161

function template is tricky, I may be not aware of the context, what's our plan for supporting it? could you give an concrete example? it seems that the current unit test doesn't cover it.

173

I believe it also works for class methods?

184

now we get a potential ODR violation in this example, maybe choose a different example?

225

The tweak is under development, do we need to hide it until it is completed?

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

25

the header seems irrelevant to me.

725

The a.h seems to be non-existed unless you explicitly set it via ExtraFiles["a.h"]?

[kadircet]

• Address comments

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

79

agreed, sent out D67748

114

we are still checking its visibility in target context, but without making use of declcontext. Updating the comment.

161

Yes we plan to support them, added a test case thanks for pointing it out.

This basically ensures that we select the correct decl in the case of template spec decls, because canonical decl is function template itself, instead of the specialization decl.

184

You are right, but we cannot get away from it by changing the example. Adding an "inline " instead, and I believe that's what we should do if we are moving a function definition to a header.

225

I wasn't planning to land it before all of its dependencies are LGTM'd, but marking it as hidden just in case.

[hokein]

mostly good to me, a few more comments.

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

157

maybe name it findTarget? I think the function is used to find a potential target decl which the definition will be moved to, Canonical doesn't provide much information here.

161

We may get a nullptr if FD is the first declaration, I think we need to handle the nullptr case, otherwise the check will crash.

184

I think not all cases will need inline, e.g. class method, or function template(?). Fix these problems is out-scope of the tweak (there is a clang-tidy check handling this case), and probably add complexity to the implementation. I'm leaning on not fixing it.

Maybe a better example is class method?

186

I'm not sure Inline function definition is a widely-known refactoring term, I think "Move function definition to declaration" is probably easier for normal C++ developers to understand?

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

729

I think we missing a basic test where a declaration is in the header and it can be moved?

Header = "void foo();"
Main = "void f^oo() { // no dependency code }"

[kadircet]

• Address comments

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

161

theoretically, a template specialization cannot be the first decl, as one needs to declare function template first.
making this more clear by changing this into a loop that finds the first forward declaration for the specialization.

184

yes clang-tidy can generate those fixes but I believe a code-action should not generate code that relies on a clang-tidy check to fix it.

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

729

isn't it covered by the next testcase ?

[hokein]

looks good with a few nits.

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

155

nit: the comment is out of date with the new change?

184

I'm still not convinced that we should fix it in the check. but we don't have to solve it out in this patch, could we remove the inline from the sample? (adding it now seems to confuse users, we don't support it yet).

clang-tools-extra/clangd/unittests/TweakTesting.h

52

nit: add a comment // file name => file content.

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

729

ah, sorry, I missed that.

[kadircet]

• Rebase on top of D67748

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

184

we basically support nothing in that sample in this patch, it is just to reflect the end-state of the code-action.
but as you wish; deleting it now, and instead adding with the next patch(that implements apply).

[kadircet]

• Rebase and update testhelpers

[kadircet]

• Revert miscommit

[kadircet]

• Use canonical decl when checking visibility

[kadircet]

• Rebase and bail out on methods inside templated classes

[thakis]

The test fails in Windows: http://45.33.8.238/win/1112/step_7.txt

Ptal, and if it takes a while to investigate please revert while you look.

clang-tools-extra/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
+  DefineInline.cpp
   ExpandAutoType.cpp
   ExpandMacro.cpp
   ExtractFunction.cpp
   ExtractVariable.cpp
   RawStringLiteral.cpp
   RemoveUsingNamespace.cpp
   SwapIfBranches.cpp
 

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

+//===--- DefineInline.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 "AST.h"
+#include "Logger.h"
+#include "Selection.h"
+#include "SourceCode.h"
+#include "XRefs.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/PrettyPrinter.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/TemplateBase.h"
+#include "clang/AST/Type.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexSymbol.h"
+#include "clang/Index/IndexingAction.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/Token.h"
+#include "clang/Sema/Lookup.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Signals.h"
+#include "llvm/Support/raw_ostream.h"
+#include <cstddef>
+#include <set>
+#include <string>
+#include <unordered_map>
+#include <vector>
+
+namespace clang {
+namespace clangd {
+namespace {
+
+// Deduces the FunctionDecl from a selection. Requires either the function body
+// or the function decl to be selected. Returns null if none of the above
+// criteria is met.
+const FunctionDecl *getSelectedFunction(const SelectionTree::Node *SelNode) {
+  const ast_type_traits::DynTypedNode &AstNode = SelNode->ASTNode;
+  if (const FunctionDecl *FD = AstNode.get<FunctionDecl>())
+    return FD;
+  if (AstNode.get<CompoundStmt>() &&
+      SelNode->Selected == SelectionTree::Complete) {
+    if (const SelectionTree::Node *P = SelNode->Parent)
+      return P->ASTNode.get<FunctionDecl>();
+  }
+  return nullptr;
+}
+
+// Checks the decls mentioned in Source are visible in the context of Target.
+// Achives that by checking declarations occur before target location in
+// translation unit or declared in the same class.
+bool checkDeclsAreVisible(const llvm::DenseSet<const Decl *> &DeclRefs,
+                          const FunctionDecl *Target, const SourceManager &SM) {
+  SourceLocation TargetLoc = Target->getLocation();
+  // To be used in visibility check below, decls in a class are visible
+  // independent of order.
+  const RecordDecl *Class = nullptr;
+  if (const auto *MD = llvm::dyn_cast<CXXMethodDecl>(Target))
+    Class = MD->getParent();
+
+  for (const auto *DR : DeclRefs) {
+    // Use canonical decl, since having one decl before target is enough.
+    const Decl *D = DR->getCanonicalDecl();
+    if (D == Target)
+      continue;
+    SourceLocation DeclLoc = D->getLocation();
+
+    // FIXME: Allow declarations from different files with include insertion.
+    if (!SM.isWrittenInSameFile(DeclLoc, TargetLoc))
+      return false;
+
+    // If declaration is before target, then it is visible.
+    if (SM.isBeforeInTranslationUnit(DeclLoc, TargetLoc))
+      continue;
+
+    // Otherwise they need to be in same class
+    if (!Class)
+      return false;
+    const RecordDecl *Parent = nullptr;
+    if (const auto *MD = llvm::dyn_cast<CXXMethodDecl>(D))
+      Parent = MD->getParent();
+    else if (const auto *FD = llvm::dyn_cast<FieldDecl>(D))
+      Parent = FD->getParent();
+    if (Parent != Class)
+      return false;
+  }
+  return true;
+}
+
+// Returns the canonical declaration for the given FunctionDecl. This will
+// usually be the first declaration in current translation unit with the
+// exception of template specialization.
+// For those we return first declaration different than the canonical one.
+// Because canonical declaration points to template decl instead of
+// specialization.
+const FunctionDecl *findTarget(const FunctionDecl *FD) {
+  auto CanonDecl = FD->getCanonicalDecl();
+  if (!FD->isFunctionTemplateSpecialization())
+    return CanonDecl;
+  // For specializations CanonicalDecl is the TemplatedDecl, which is not the
+  // target we want to inline into. Instead we traverse previous decls to find
+  // the first forward decl for this specialization.
+  auto PrevDecl = FD;
+  while (PrevDecl->getPreviousDecl() != CanonDecl) {
+    PrevDecl = PrevDecl->getPreviousDecl();
+    assert(PrevDecl && "Found specialization without template decl");
+  }
+  return PrevDecl;
+}
+
+/// Moves definition of a function/method to its declaration location.
+/// Before:
+/// a.h:
+///   void foo();
+///
+/// a.cc:
+///   void foo() { return; }
+///
+/// ------------------------
+/// After:
+/// a.h:
+///   void foo() { return; }
+///
+/// a.cc:
+///
+class DefineInline : public Tweak {
+public:
+  const char *id() const override final;
+
+  Intent intent() const override { return Intent::Refactor; }
+  std::string title() const override {
+    return "Move function body to declaration";
+  }
+  bool hidden() const override { return true; }
+
+  // Returns true when selection is on a function definition that does not
+  // make use of any internal symbols.
+  bool prepare(const Selection &Sel) override {
+    const SelectionTree::Node *SelNode = Sel.ASTSelection.commonAncestor();
+    if (!SelNode)
+      return false;
+    Source = getSelectedFunction(SelNode);
+    if (!Source || !Source->isThisDeclarationADefinition())
+      return false;
+    // Only the last level of template parameter locations are not kept in AST,
+    // so if we are inlining a method that is in a templated class, there is no
+    // way to verify template parameter names. Therefore we bail out.
+    if (auto *MD = llvm::dyn_cast<CXXMethodDecl>(Source)) {
+      if (MD->getParent()->isTemplated())
+        return false;
+    }
+
+    Target = findTarget(Source);
+    if (Target == Source) {
+      // The only declaration is Source. No other declaration to move function
+      // body.
+      // FIXME: If we are in an implementation file, figure out a suitable
+      // location to put declaration. Possibly using other declarations in the
+      // AST.
+      return false;
+    }
+
+    // Check if the decls referenced in function body are visible in the
+    // declaration location.
+    if (!checkDeclsAreVisible(getNonLocalDeclRefs(Sel.AST, Source), Target,
+                              Sel.AST.getSourceManager()))
+      return false;
+
+    return true;
+  }
+
+  Expected<Effect> apply(const Selection &Sel) override {
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "Not implemented yet");
+  }
+
+private:
+  const FunctionDecl *Source = nullptr;
+  const FunctionDecl *Target = nullptr;
+};
+
+REGISTER_TWEAK(DefineInline);
+
+} // namespace
+} // namespace clangd
+} // namespace clang

clang-tools-extra/clangd/unittests/TweakTesting.h

 //===--- TweakTesting.h - Test helpers for refactoring actions ---*- 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_UNITTESTS_CLANGD_TWEAKTESTING_H
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TWEAKTESTING_H
 
 #include "TestTU.h"
-#include "gtest/gtest.h"
+#include "llvm/ADT/StringMap.h"
 #include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include <string>
 
 namespace clang {
 namespace clangd {
 
 // Fixture base for testing tweaks. Intended to be subclassed for each tweak.
 //
 // Usage:
 // TWEAK_TEST(ExpandAutoType);
enum CodeContext {
     // Code snippet is placed directly into the source file. e.g. a declaration.
     File,
     // Snippet will appear within a function body. e.g. a statement.
     Function,
     // Snippet is an expression.
     Expression,
   };
 
+  // Mapping from file name to contents.
+  llvm::StringMap<std::string> ExtraFiles;
+
 protected:
   TweakTest(const char *TweakID) : TweakID(TweakID) {}
 
   // Contents of a header file to be implicitly included.
   // This typically contains declarations that will be used for a set of related
   // testcases.
   std::string Header;
 

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

if (A.points().size() != 0) {
     SelectionRng = Range{A.point(), A.point()};
   } else {
     SelectionRng = A.range();
   }
   return {cantFail(positionToOffset(A.code(), SelectionRng.start)),
           cantFail(positionToOffset(A.code(), SelectionRng.end))};
 }
 
-MATCHER_P3(TweakIsAvailable, TweakID, Ctx, Header,
+MATCHER_P4(TweakIsAvailable, TweakID, Ctx, Header, ExtraFiles,
            (TweakID + (negation ? " is unavailable" : " is available")).str()) {
   std::string WrappedCode = wrap(Ctx, arg);
   Annotations Input(WrappedCode);
   auto Selection = rangeOrPoint(Input);
   TestTU TU;
   TU.HeaderCode = Header;
   TU.Code = Input.code();
+  TU.AdditionalFiles = std::move(ExtraFiles);
   ParsedAST AST = TU.build();
   Tweak::Selection S(AST, Selection.first, Selection.second);
   auto PrepareResult = prepareTweak(TweakID, S);
   if (PrepareResult)
     return true;
   llvm::consumeError(PrepareResult.takeError());
   return false;
 }
std::string TweakTest::apply(llvm::StringRef MarkedCode) const {
   auto ApplyEdit = Result->ApplyEdits.begin()->second;
   if (auto NewText = ApplyEdit.apply())
     return unwrap(Context, *NewText);
   else
     return "bad edits: " + llvm::toString(NewText.takeError());
 }
 
 ::testing::Matcher<llvm::StringRef> TweakTest::isAvailable() const {
-  return TweakIsAvailable(llvm::StringRef(TweakID), Context, Header); 
+  return TweakIsAvailable(llvm::StringRef(TweakID), Context, Header,
+                          ExtraFiles);
 }
 
 std::vector<std::string> TweakTest::expandCases(llvm::StringRef MarkedCode) {
   Annotations Test(MarkedCode);
   llvm::StringRef Code = Test.code();
   std::vector<std::string> Cases;
   for (const auto& Point : Test.points()) {
     size_t Offset = llvm::cantFail(positionToOffset(Code, Point));

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

 #include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock-matchers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <cassert>
+#include <string>
+#include <vector>
 
 using ::testing::AllOf;
 using ::testing::HasSubstr;
 using ::testing::StartsWith;
 
 namespace clang {
 namespace clangd {
 namespace {
 
 TEST(FileEdits, AbsolutePath) {
   auto RelPaths = {"a.h", "foo.cpp", "test/test.cpp"};
 
   llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> MemFS(
       new llvm::vfs::InMemoryFileSystem);
   MemFS->setCurrentWorkingDirectory(testRoot());
   for (auto Path : RelPaths)
     MemFS->addFile(Path, 0, llvm::MemoryBuffer::getMemBuffer("", Path));
   FileManager FM(FileSystemOptions(), MemFS);
   DiagnosticsEngine DE(new DiagnosticIDs, new DiagnosticOptions);
   SourceManager SM(DE, FM);
 
   for (auto Path : RelPaths) {
std::pair<llvm::StringRef /*Input*/, llvm::StringRef /*Expected*/> Cases[] = {
     )cpp"},
       {// Nested namespace: Fully qualify ident from inner ns.
        R"cpp(
       namespace aa { namespace bb { struct map {}; }}
       using namespace aa::b^b;
       int main() {
         map m;
       }
     )cpp",
        R"cpp(
       namespace aa { namespace bb { struct map {}; }}
       
       int main() {
         aa::bb::map m;
       }
     )cpp"},
       {// Nested namespace: Fully qualify ident from inner ns.
        R"cpp(
       namespace aa { namespace bb { struct map {}; }}
       using namespace a^a;
       int main() {
)cpp",
       int main() {
         std::vector V;
       }
     )cpp"}};
   for (auto C : Cases)
     EXPECT_EQ(C.second, apply(C.first)) << C.first;
 }
 
+TWEAK_TEST(DefineInline);
+TEST_F(DefineInlineTest, TriggersOnFunctionDecl) {
+  // Basic check for function body and signature.
+  EXPECT_AVAILABLE(R"cpp(
+  class Bar {
+    void baz();
+  };
+
+  [[void [[Bar::[[b^a^z]]]]() [[{
+    return;
+  }]]]]
+
+  void foo();
+  [[void [[f^o^o]]() [[{
+    return;
+  }]]]]
+  )cpp");
+
+  EXPECT_UNAVAILABLE(R"cpp(
+  // Not a definition
+  vo^i[[d^ ^f]]^oo();
+
+  [[vo^id ]]foo[[()]] {[[
+    [[(void)(5+3);
+    return;]]
+  }]]
+  )cpp");
+}
+
+TEST_F(DefineInlineTest, NoForwardDecl) {
+  Header = "void bar();";
+  EXPECT_UNAVAILABLE(R"cpp(
+  void bar() {
+    return;
+  }
+  // FIXME: Generate a decl in the header.
+  void fo^o() {
+    return;
+  })cpp");
+}
+
+TEST_F(DefineInlineTest, ReferencedDecls) {
+  EXPECT_AVAILABLE(R"cpp(
+    void bar();
+    void foo(int test);
+
+    void fo^o(int baz) {
+      int x = 10;
+      bar();
+    })cpp");
+
+  // Internal symbol usage.
+  Header = "void foo(int test);";
+  EXPECT_UNAVAILABLE(R"cpp(
+    void bar();
+    void fo^o(int baz) {
+      int x = 10;
+      bar();
+    })cpp");
+
+  // Becomes available after making symbol visible.
+  Header = "void bar();" + Header;
+  EXPECT_AVAILABLE(R"cpp(
+    void fo^o(int baz) {
+      int x = 10;
+      bar();
+    })cpp");
+
+  // FIXME: Move declaration below bar to make it visible.
+  Header.clear();
+  EXPECT_UNAVAILABLE(R"cpp(
+    void foo();
+    void bar();
+
+    void fo^o() {
+      bar();
+    })cpp");
+
+  // Order doesn't matter within a class.
+  EXPECT_AVAILABLE(R"cpp(
+    class Bar {
+      void foo();
+      void bar();
+    };
+
+    void Bar::fo^o() {
+      bar();
+    })cpp");
+
+  // FIXME: Perform include insertion to make symbol visible.
+  ExtraFiles["a.h"] = "void bar();";
+  Header = "void foo(int test);";
+  EXPECT_UNAVAILABLE(R"cpp(
+    #include "a.h"
+    void fo^o(int baz) {
+      int x = 10;
+      bar();
+    })cpp");
+}
+
+TEST_F(DefineInlineTest, TemplateSpec) {
+  EXPECT_UNAVAILABLE(R"cpp(
+    template <typename T> void foo();
+    template<> void foo<char>();
+
+    template<> void f^oo<int>() {
+    })cpp");
+  EXPECT_UNAVAILABLE(R"cpp(
+    template <typename T> void foo();
+
+    template<> void f^oo<int>() {
+    })cpp");
+  EXPECT_UNAVAILABLE(R"cpp(
+    template <typename T> struct Foo { void foo(); };
+
+    template <typename T> void Foo<T>::f^oo() {
+    })cpp");
+  EXPECT_AVAILABLE(R"cpp(
+    template <typename T> void foo();
+    void bar();
+    template <> void foo<int>();
+
+    template<> void f^oo<int>() {
+      bar();
+    })cpp");
+}
+
+TEST_F(DefineInlineTest, CheckForCanonDecl) {
+  EXPECT_UNAVAILABLE(R"cpp(
+    void foo();
+
+    void bar() {}
+    void f^oo() {
+      // This bar normally refers to the definition just above, but it is not
+      // visible from the forward declaration of foo.
+      bar();
+    })cpp");
+  // Make it available with a forward decl.
+  EXPECT_AVAILABLE(R"cpp(
+    void bar();
+    void foo();
+
+    void bar() {}
+    void f^oo() {
+      bar();
+    })cpp");
+}
+
+TEST_F(DefineInlineTest, UsingShadowDecls) {
+  EXPECT_UNAVAILABLE(R"cpp(
+  namespace ns1 { void foo(int); }
+  namespace ns2 { void foo(int*); }
+  template <typename T>
+  void bar();
+
+  using ns1::foo;
+  using ns2::foo;
+
+  template <typename T>
+  void b^ar() {
+    foo(T());
+  })cpp");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang