[mlir][lsp] Use ResultGroupDefinition struct

[Mogball]

Summary

This struct was added and was intended to be used, but it was missed in the original patch.

[rriddle]

Nice catch!

[rriddle]

Can you add a note in the description that this was already intended, but missed originally?

[Mogball]

For sure.

mlir/include/mlir/Parser/AsmParserState.h

struct SMDefinition {
     /// The source location of all uses of the definition.
     SmallVector<llvm::SMRange> uses;
   };
 
   /// This class represents the information for an operation definition within
   /// an input file.
   struct OperationDefinition {
     struct ResultGroupDefinition {
+      ResultGroupDefinition(unsigned index, llvm::SMRange loc)
+          : startIndex(index), definition(loc) {}
+
       /// The result number that starts this group.
       unsigned startIndex;
       /// The source definition of the result group.
       SMDefinition definition;
     };
 
     OperationDefinition(Operation *op, llvm::SMRange loc, llvm::SMLoc endLoc)
         : op(op), loc(loc), scopeLoc(loc.Start, endLoc) {}
 
     /// The operation representing this definition.
     Operation *op;
 
     /// The source location for the operation, i.e. the location of its name.
     llvm::SMRange loc;
 
     /// The full source range of the operation definition, i.e. a range
     /// encompassing the start and end of the full operation definition.
     llvm::SMRange scopeLoc;
 
     /// Source definitions for any result groups of this operation.
-    SmallVector<std::pair<unsigned, SMDefinition>> resultGroups;
+    SmallVector<ResultGroupDefinition> resultGroups;
 
     /// If this operation is a symbol operation, this vector contains symbol
     /// uses of this operation.
     SmallVector<llvm::SMRange> symbolUses;
   };
 
   /// This class represents the information for a block definition within the
   /// input file.

mlir/lib/Parser/AsmParserState.cpp

if (OpResult result = value.dyn_cast<OpResult>()) {
     }
 
     // If a definition does exist, locate the value's result group and add the
     // use. The result groups are ordered by increasing start index, so we just
     // need to find the last group that has a smaller/equal start index.
     unsigned resultNo = result.getResultNumber();
     OperationDefinition &def = *impl->operations[existingIt->second];
     for (auto &resultGroup : llvm::reverse(def.resultGroups)) {
-      if (resultNo >= resultGroup.first) {
+      if (resultNo >= resultGroup.startIndex) {
         for (llvm::SMLoc loc : locations)
-          resultGroup.second.uses.push_back(convertIdLocToRange(loc));
+          resultGroup.definition.uses.push_back(convertIdLocToRange(loc));
         return;
       }
     }
     llvm_unreachable("expected valid result group for value use");
   }
 
   // Otherwise, this is a block argument.
   BlockArgument arg = value.cast<BlockArgument>();

mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp

auto containsPosition = [&](const AsmParserState::SMDefinition &def) {
     return true;
   };
 
   // Check all definitions related to operations.
   for (const AsmParserState::OperationDefinition &op : asmState.getOpDefs()) {
     if (contains(op.loc, posLoc))
       return collectLocationsFromLoc(op.op->getLoc(), locations, uri);
     for (const auto &result : op.resultGroups)
-      if (containsPosition(result.second))
+      if (containsPosition(result.definition))
         return collectLocationsFromLoc(op.op->getLoc(), locations, uri);
     for (const auto &symUse : op.symbolUses) {
       if (contains(symUse, posLoc)) {
         locations.push_back(getLocationFromLoc(sourceMgr, op.loc, uri));
         return collectLocationsFromLoc(op.op->getLoc(), locations, uri);
       }
     }
   }
void MLIRDocument::findReferencesOf(const lsp::URIForFile &uri,
   };
 
   llvm::SMLoc posLoc = getPosFromLoc(sourceMgr, pos);
 
   // Check all definitions related to operations.
   for (const AsmParserState::OperationDefinition &op : asmState.getOpDefs()) {
     if (contains(op.loc, posLoc)) {
       for (const auto &result : op.resultGroups)
-        appendSMDef(result.second);
+        appendSMDef(result.definition);
       for (const auto &symUse : op.symbolUses)
         if (contains(symUse, posLoc))
           references.push_back(getLocationFromLoc(sourceMgr, symUse, uri));
       return;
     }
     for (const auto &result : op.resultGroups)
-      if (isDefOrUse(result.second, posLoc))
-        return appendSMDef(result.second);
+      if (isDefOrUse(result.definition, posLoc))
+        return appendSMDef(result.definition);
     for (const auto &symUse : op.symbolUses) {
       if (!contains(symUse, posLoc))
         continue;
       for (const auto &symUse : op.symbolUses)
         references.push_back(getLocationFromLoc(sourceMgr, symUse, uri));
       return;
     }
   }
for (const AsmParserState::OperationDefinition &op : asmState.getOpDefs()) {
     // Check if the position points at the symbol name.
     for (auto &use : op.symbolUses)
       if (contains(use, posLoc))
         return buildHoverForOperation(use, op);
 
     // Check if the position points at a result group.
     for (unsigned i = 0, e = op.resultGroups.size(); i < e; ++i) {
       const auto &result = op.resultGroups[i];
-      if (!isDefOrUse(result.second, posLoc, &hoverRange))
+      if (!isDefOrUse(result.definition, posLoc, &hoverRange))
         continue;
 
       // Get the range of results covered by the over position.
-      unsigned resultStart = result.first;
-      unsigned resultEnd =
-          (i == e - 1) ? op.op->getNumResults() : op.resultGroups[i + 1].first;
+      unsigned resultStart = result.startIndex;
+      unsigned resultEnd = (i == e - 1) ? op.op->getNumResults()
+                                        : op.resultGroups[i + 1].startIndex;
       return buildHoverForOperationResult(hoverRange, op.op, resultStart,
                                           resultEnd, posLoc);
     }
   }
 
   // Check to see if the hover is over a block argument.
   for (const AsmParserState::BlockDefinition &block : asmState.getBlockDefs()) {
     if (isDefOrUse(block.definition, posLoc, &hoverRange))