+1. The fact that APIs named “getLineNumber” and “getLineAndColumn” do subtly different things is pretty awful.
I like “presumed” better than “virtual”, but it’s not a big deal to me either way. Ben > On Mar 16, 2017, at 3:20 PM, Jordan Rose via swift-dev <swift-dev@swift.org> > wrote: > > FWIW this sounds like a great idea to me. Clang calls these "presumed > locations" instead of "virtual locations" but either way is fine with me. I'd > like one of the SourceKit folks to weigh in, though, since they're the most > involved in operations at this layer. (That'd be Argyrios Kyrtzidis, Ben > Langmuir, Xi Ge, David Farler, or Nathan Hawes.) > > Jordan > > >> On Mar 15, 2017, at 23:23, rintaro ishizaki via swift-dev >> <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote: >> >> Hi all, >> >> I would like to propose to reorganize methods for retrieving filename, line >> and column from SourceLoc. >> >> Motiavion >> >> When we want to retrieve filename, line and column from SourceLoc, we have >> several ways to do that. >> >> Filename: >> SourceManager::getBufferIdentifierForLoc >> SourceManager::getIdentifierForBuffer >> SourceFile::getFilename >> LoadedFile::getFilename >> llvm::MemoryBuffer::getBufferIdentifier >> >> Line and Column: >> SourceManager::getLineAndColumn >> SourceManager::getLineNumber >> >> Some of them respect #sourceLocation directive, but some not: >> >> Virtual (respect #sourceLocation): >> SourceManager::getBufferIdentifierForLoc >> SourceManager::getLineAndColumn >> >> Actual: >> SourceManager::getIdentifierForBuffer >> SourceFile::getFilename >> LoadedFile::getFilename >> llvm::MemoryBuffer::getBufferIdentifier >> SourceManager::getLineNumber >> >> Mixed use of them causes errors like >> https://github.com/apple/swift/pull/5425 >> <https://github.com/apple/swift/pull/5425> >> Since current method namings are unclear about this, I think it's very error >> prone. Also, we currently don't have any method for *real* getLineAndColumn. >> When we want to retrieve the real line and column, we have use >> getLineNumber(loc) for the line, and getLineAndColumn(loc).second for the >> column. Most of our codebase doesn't bother to do that, though. >> >> I suspect that Xcode crash when using #sourceLocation in PlayGround is >> probably caused by this kind of problem; i.e. Xcode expects "real" line and >> column, but the compiler or sourcekit returns virtual one. >> >> >> Proposed Solution >> >> Reorganize SourceManager methods. >> >> We need methods for "real" one, and "virtual" one. And they need to have >> distinct name for that. >> >> ** RFC for method names ** >> >> class SourceManager { >> >> public: >> >> /// @{ >> /// Real buffer-name, line, and column. >> /// These methods does not respect '#sourceLocation' >> >> /// Returns the identifier string for the buffer with the given ID. >> StringRef >> getIdentifierForBuffer(unsigned BufferID) const; >> >> /// Returns the buffer identifier string for the Loc. >> /// If 'BufferID' is provided 'Loc' must come from that buffer. >> StringRef >> getIdentifierForBuffer(SourceLoc Loc, unsigned BufferID = 0) const; >> >> /// Returns the pair of line and column in the buffer. >> /// If 'BufferID' is provided 'Loc' must come from that buffer. >> std::pair<unsigned, unsigned> >> getLineAndColumnInBuffer(SourceLoc Loc, unsigned BufferID = 0) const; >> >> /// } >> >> /// @{ >> /// Virtual buffer-name, line, and column. >> /// These methods does respect '#sourceLocation' >> >> /// Returns the virtual filename string for the Loc. >> /// If 'BufferID' is provided 'Loc' must come from that buffer. >> StringRef >> getVirtualFilenameForLoc(SourceLoc Loc, unsigned BufferID = 0) const; >> >> /// Returns the pair of line and column in the buffer >> /// respecting #sourceLocation directive. >> /// If 'BufferID' is provided 'Loc' must come from that buffer. >> std::pair<unsigned, unsigned> >> getVirtutalLineAndColumnForLoc(SourceLoc Loc, unsigned BufferID = 0) const; >> >> /// } >> >> Roughly: >> >> Remove: getLineNumber >> ASIS: getIdentifierForBuffer(BufferID) >> Add: getIdentifierForBuffer(Loc) // overload >> Add: getLineAndColumnInBuffer >> Rename: getBufferIdentifierForLoc => getVirtualFilenameForLoc >> Rename: getLineAndColumn => getVirtutalLineAndColumnForLoc >> >> >> Audit every current usage of these methods >> >> We already have several mix usage of them. For example: >> >> getLineNumber & getLineAndColumn >> https://github.com/apple/swift/blob/6293546/lib/AST/RawComment.cpp#L61-L63 >> <https://github.com/apple/swift/blob/6293546/lib/AST/RawComment.cpp#L61-L63> >> >> getIdentifierForBuffer & getLineAndColumn >> https://github.com/apple/swift/blob/6293546/lib/Frontend/SerializedDiagnosticConsumer.cpp#L242 >> >> <https://github.com/apple/swift/blob/6293546/lib/Frontend/SerializedDiagnosticConsumer.cpp#L242> >> https://github.com/apple/swift/blob/6293546/lib/Frontend/SerializedDiagnosticConsumer.cpp#L451 >> >> <https://github.com/apple/swift/blob/6293546/lib/Frontend/SerializedDiagnosticConsumer.cpp#L451> >> >> SourceFile::getFilename & getLineAndColumn >> https://github.com/apple/swift/blob/6293546/lib/SILGen/SILGenProfiling.cpp#L706 >> >> <https://github.com/apple/swift/blob/6293546/lib/SILGen/SILGenProfiling.cpp#L706> >> https://github.com/apple/swift/blob/6293546/lib/SILGen/SILGenProfiling.cpp#L485-L486 >> >> <https://github.com/apple/swift/blob/6293546/lib/SILGen/SILGenProfiling.cpp#L485-L486> >> >> Also, I believe there are some places where we are using "virtutal" one even >> though we should use "real" one, and vise versa. >> >> I'm currently making a complete list of them. We should audit them all, and >> choose the correct methods. However, it's not clear for me that which one to >> use which methods; for example, I have no idea whether CoverageMapping >> intended to point the real locations or virtual. >> >> >> That's all I currently have. >> Could you please share your opinions on this before I proceed? >> >> -- >> Rintaro >> _______________________________________________ >> swift-dev mailing list >> swift-dev@swift.org <mailto:swift-dev@swift.org> >> https://lists.swift.org/mailman/listinfo/swift-dev > > _______________________________________________ > swift-dev mailing list > swift-dev@swift.org > https://lists.swift.org/mailman/listinfo/swift-dev
_______________________________________________ swift-dev mailing list swift-dev@swift.org https://lists.swift.org/mailman/listinfo/swift-dev
