[clangd] Remove the overflow log.

[hokein]

Summary

LLVM codebase has generated files (all are build/Target/XXX/*.inc) that
exceed the MaxLine & MaxColumn. Printing these log would be noisy.

[sammccall]

After looking at the examples, I'm reassured that we don't really care about tracking precise locations of those references :-)

I'd suggest adding the logging just to where the field is *read* in XRefs.cpp (check if it's max-value).
That would cover a bunch of the cases where an incorrect location is likely to be visible.

[hokein]

Add log in XRefs.cpp.

[sammccall]

Some nitty ideas about the log message, but whatever you think is best.

clangd/XRefs.cpp

43

Log message could use more context. And I think it'd be really useful to print the whole location here.

bikeshed: you could add a method bool Position::hasOverflow(), and then write below

if (LSPLoc.range.start.hasOverflow() || LSPLoc.range.end.hasOverflow())
  log("Possible overflow in symbol location: {0}", LSPLoc);

Distinguishing between line/column overflow isn't important if you're printing the full range anyway.

[hokein]

Log the whole location when overflow happens.

[hokein]

clangd/XRefs.cpp

43

This is neat.

[hokein]

Minor cleanup.

clangd/XRefs.cpp

if (const auto *TD = dyn_cast<TagDecl>(D))
     return TD->getDefinition();
   else if (const auto *VD = dyn_cast<VarDecl>(D))
     return VD->getDefinition();
   else if (const auto *FD = dyn_cast<FunctionDecl>(D))
     return FD->getDefinition();
   return nullptr;
 }
 
+void logIfOverflow(const SymbolLocation Loc) {
+  if (Loc.Start.hasOverflow() || Loc.End.hasOverflow())
+    log("Possible overflow in symbol location: {0}", Loc);
+}
+
 // Convert a SymbolLocation to LSP's Location.
 // HintPath is used to resolve the path of URI.
 // FIXME: figure out a good home for it, and share the implementation with
 // FindSymbols.
 llvm::Optional<Location> toLSPLocation(const SymbolLocation &Loc,
                                        llvm::StringRef HintPath) {
   if (!Loc)
     return llvm::None;
   auto Uri = URI::parse(Loc.FileURI);
   if (!Uri) {
     log("Could not parse URI: {0}", Loc.FileURI);
     return llvm::None;
   }
   auto Path = URI::resolve(*Uri, HintPath);
   if (!Path) {
     log("Could not resolve URI: {0}", Loc.FileURI);
     return llvm::None;
   }
   Location LSPLoc;
   LSPLoc.uri = URIForFile(*Path);
   LSPLoc.range.start.line = Loc.Start.line();
   LSPLoc.range.start.character = Loc.Start.column();
   LSPLoc.range.end.line = Loc.End.line();
   LSPLoc.range.end.character = Loc.End.column();
+  logIfOverflow(Loc);
   return LSPLoc;
 }
 
 struct MacroDecl {
   StringRef Name;
   const MacroInfo *Info;
 };
 

clangd/index/Index.h

struct SymbolLocation {
   // Position is encoded into 32 bits to save space.
   // If Line/Column overflow, the value will be their maximum value.
   struct Position {
     void setLine(uint32_t Line);
     uint32_t line() const { return Line; }
     void setColumn(uint32_t Column);
     uint32_t column() const { return Column; }
 
+    bool hasOverflow() const {
+      return Line >= MaxLine || Column >= MaxColumn;
+    }
+
     static constexpr uint32_t MaxLine = (1 << 20) - 1;
     static constexpr uint32_t MaxColumn = (1 << 12) - 1;
 
     // Clients should use getters and setters to access these members.
     // FIXME: hide these members.
     uint32_t Line : 20; // 0-based
     // Using UTF-16 code units.
     uint32_t Column : 12; // 0-based

clangd/index/Index.cpp

 namespace clang {
 namespace clangd {
 using namespace llvm;
 
 constexpr uint32_t SymbolLocation::Position::MaxLine;
 constexpr uint32_t SymbolLocation::Position::MaxColumn;
 void SymbolLocation::Position::setLine(uint32_t L) {
   if (L > MaxLine) {
-    log("Set an overflowed Line {0}", L);
     Line = MaxLine;
     return;
   }
   Line = L;
 }
 void SymbolLocation::Position::setColumn(uint32_t Col) {
   if (Col > MaxColumn) {
-    log("Set an overflowed Column {0}", Col);
     Column = MaxColumn;
     return;
   }
   Column = Col;
 }
 
 raw_ostream &operator<<(raw_ostream &OS, const SymbolLocation &L) {
   if (!L)

unittests/clangd/IndexTests.cpp

 TEST(SymbolLocation, Position) {
   using Position = SymbolLocation::Position;
   Position Pos;
 
   Pos.setLine(1);
   EXPECT_EQ(1u, Pos.line());
   Pos.setColumn(2);
   EXPECT_EQ(2u, Pos.column());
+  EXPECT_FALSE(Pos.hasOverflow());
 
   Pos.setLine(Position::MaxLine + 1); // overflow
+  EXPECT_TRUE(Pos.hasOverflow());
   EXPECT_EQ(Pos.line(), Position::MaxLine);
+  Pos.setLine(1); // reset the overflowed line.
+
   Pos.setColumn(Position::MaxColumn + 1); // overflow
+  EXPECT_TRUE(Pos.hasOverflow());
   EXPECT_EQ(Pos.column(), Position::MaxColumn);
 }
 
 TEST(SymbolSlab, FindAndIterate) {
   SymbolSlab::Builder B;
   B.insert(symbol("Z"));
   B.insert(symbol("Y"));
   B.insert(symbol("X"));