AaronBallman wrote: > > Sure, but I was thinking more that we do want to canonicalize paths like > > `blah\src/foo.cc` to `blah\src\foo.cc` on Windows. so there are consistent > > Windows path separators being used. Perhaps my use of "canonicalize" isn't > > right. :-) > > Yeah, that would be nice, but canonicalisation afaik always involves > conversion to an absolute path; I don’t know if we have a function that > normalises paths like that. > > > Agreed; I mostly was suggesting that whatever path ends up being shorter, > > it would still be nice to ensure the path printed out uses consistent > > separators (so the length stays the same, it's just a more natural-looking > > way to express the file for that particular file system). > > I honestly do wonder if it wouldn’t make more sense to go back to doing this > in `SourceManager`—or somewhere in the diagnostics machinery; what with all > of the tests that are affected by this, the fact that we probably want a > separate flag for this anyway, and the fact that it’s a bit > ‘non-deterministic’ in the sense that I don’t think we really care what > exaclty we end up printing so long as it’s 1. a valid path, and 2. no larger > than the path we started with (and I suppose 3. making path separators more > consistent would also be nice). > > I feel like this change is really only relevant for the very specific > situation of printing diagnostics to the terminal
I was thinking it's also useful for `-###` and `-v` output, crash reports, `-ast-dump` output... > —an IDE can just point you to the error directly, and e.g. SARIF isn’t > user-facing either way so I don’t think you’d care about simplified paths in > either situation. I think SARIF already has to care, somewhat. IIRC the paths in SARIF files have to follow a particular RFC (RFC 3986 I think?) and that will dictate how paths are referenced. > With how this has turned out, doing this (close to) where we print the > diagnostics to me seems like it would be less of a hassle... > > Either way though, I think we do need a separate flag that enables/disables > this. Yeah, I think the flag will be necessary, though hopefully we can default the flag to enable it. https://github.com/llvm/llvm-project/pull/148745 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
