Change the error message when `-split-input-file` is used with mlir-opt to make it recognizable by IDEs

[mehdi_amini]

Summary

By adding the line number of the split point immediately after the file
name (separated by :) this is recognized by various tool as a proper
location.

Ideally we would want to point to the line of the error, but that would
require some very invasive changes I suspect.

[mehdi_amini]

update

[jpienaar]

Could you add an example output? I'm trying to think what it would look like (I was wondering yesterday hoe invasive it would be, if the input could be just an offset into the main buffer but I didn't do any digging ... May be something needed in underlying file manager to allow it)

[mehdi_amini]

It'll show /llvm-project/mlir/test/IR/invalid.mlir:14+:3:27: error: unexpected error: invalid tensor element type

[jpienaar]

And the IDE would jump to /llvm-project/mlir/test/IR/invalid.mlir:14 ? Or it would actually go to the location? I'm just thinking if a user would know this is relative offset and the location is the split being pointed to or if something like for split at /llvm-project/mlir/test/IR/invalid.mlir:14 within split :3:27 would give same IDE behavior while self-documenting?

[rriddle]

Any update here?

[mehdi_amini]

In D93363#2458330, @jpienaar wrote:

And the IDE would jump to /llvm-project/mlir/test/IR/invalid.mlir:14 ?

Yes, this is the beginning of the split thought.

I'm just thinking if a user would know this is relative offset and the location is the split being pointed to or if something like for split at /llvm-project/mlir/test/IR/invalid.mlir:14 within split :3:27 would give same IDE behavior while self-documenting?

This would be nice, but I think we need a more intrusive change for that.

[jpienaar]

In D93363#2534689, @mehdi_amini wrote:

In D93363#2458330, @jpienaar wrote:

And the IDE would jump to /llvm-project/mlir/test/IR/invalid.mlir:14 ?

Yes, this is the beginning of the split thought.

I'm just thinking if a user would know this is relative offset and the location is the split being pointed to or if something like for split at /llvm-project/mlir/test/IR/invalid.mlir:14 within split :3:27 would give same IDE behavior while self-documenting?

This would be nice, but I think we need a more intrusive change for that.

Would it? Doesn't my suggestion just require changing + to within split ?

[mehdi_amini]

In D93363#2542050, @jpienaar wrote:

I'm just thinking if a user would know this is relative offset and the location is the split being pointed to or if something like for split at /llvm-project/mlir/test/IR/invalid.mlir:14 within split :3:27 would give same IDE behavior while self-documenting?

This would be nice, but I think we need a more intrusive change for that.

Would it? Doesn't my suggestion just require changing + to within split ?

I am not sure we're reading it the same way.
Assuming an error happens at line 17 in the file, for a split at line 14 (and column 27), with this change we'd display: /llvm-project/mlir/test/IR/invalid.mlir:14+:3:27: error:.
What you suggest would be /llvm-project/mlir/test/IR/invalid.mlir:14 within split 3:27: error:, what I'd like ideally would be /llvm-project/mlir/test/IR/invalid.mlir:17:27 within split at 14: error:
I.e. I'd expect within split to point at the split point, not introduce the offset within the split.

[jpienaar]

What you suggest would be /llvm-project/mlir/test/IR/invalid.mlir:14 within split 3:27: error:, what I'd like ideally would be /llvm-project/mlir/test/IR/invalid.mlir:17:27 within split at 14: error:

Exactly (well I had a prefix to the filename too, but beyond that) and I agree with the ideal even though I'd be happy with just /llvm-project/mlir/test/IR/invalid.mlir:17:27: error:

Assuming an error happens at line 17 in the file, for a split at line 14 (and column 27), with this change we'd display: /llvm-project/mlir/test/IR/invalid.mlir:14+:3:27: error:

My initial point was that + is a bit magical there - if I got file:14+:3:27 I would need to look it up (esp with : before 3). So something that is locally documented (even if verbose) seemed preferred as this is probably not common

post split at llvm-project/mlir/test/IR/invalid.mlir:14 with offset:3:27: error:

[mehdi_amini]

Tweak the wording/format to be more verbose/self-explanatory

mlir/lib/Support/ToolUtilities.cpp

mlir::splitAndProcessBuffer(std::unique_ptr<llvm::MemoryBuffer> originalBuffer,
   fileSourceMgr.AddNewSourceBuffer(std::move(originalBuffer), llvm::SMLoc());
 
   // Process each chunk in turn.
   bool hadFailure = false;
   for (auto &subBuffer : sourceBuffers) {
     auto splitLoc = llvm::SMLoc::getFromPointer(subBuffer.data());
     unsigned splitLine = fileSourceMgr.getLineAndColumn(splitLoc).first;
     auto subMemBuffer = llvm::MemoryBuffer::getMemBufferCopy(
-        subBuffer, origMemBuffer->getBufferIdentifier() +
-                       Twine(" split at line #") + Twine(splitLine));
+        subBuffer, Twine("within split at ") +
+                       origMemBuffer->getBufferIdentifier() + ":" +
+                       Twine(splitLine) + " offset ");
     if (failed(processChunkBuffer(std::move(subMemBuffer), os)))
       hadFailure = true;
   }
 
   // If any fails, then return a failure of the tool.
   return failure(hadFailure);
 }