Filename, Line and Column for SourceLoc

rintaro · 2017-03-16T06:23:56.000Z

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
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

github.com

apple/swift/blob/6293546/lib/AST/RawComment.cpp#L61-L63


      
              EndLine(SourceMgr.getLineNumber(Range.getEnd())) {
          auto StartLineAndColumn = SourceMgr.getLineAndColumn(Range.getStart());
          StartLine = StartLineAndColumn.first;

getIdentifierForBuffer & getLineAndColumn

github.com

apple/swift/blob/6293546/lib/Frontend/SerializedDiagnosticConsumer.cpp#L242


      
            if (!Loc.isValid()) {
              // Emit a "sentinel" location.
              Record.push_back((unsigned)0); // File.
              Record.push_back((unsigned)0); // Line.
              Record.push_back((unsigned)0); // Column.
              Record.push_back((unsigned)0); // Offset.
              return;
            }
          
          
  unsigned line, col;
            std::tie(line, col) = SM.getLineAndColumn(Loc);
          
          
  Record.push_back(getEmitFile(Filename));
            Record.push_back(line);
            Record.push_back(col);
            Record.push_back(0);
          }
          
          
void SerializedDiagnosticConsumer::addRangeToRecord(CharSourceRange Range,
                                                              SourceManager &SM,
                                                              StringRef Filename,

github.com

apple/swift/blob/6293546/lib/Frontend/SerializedDiagnosticConsumer.cpp#L451


      
                              StringRef Text,
                              const DiagnosticInfo &Info) {
          
          
// Emit the diagnostic to bitcode.
          llvm::BitstreamWriter &Stream = State->Stream;
          RecordData &Record = State->Record;
          AbbreviationMap &Abbrevs = State->Abbrevs;
          
          
StringRef filename = "";
          if (Loc.isValid())
            filename = SM.getIdentifierForBuffer(SM.findBufferContainingLoc(Loc));
          
          
// Emit the RECORD_DIAG record.
          Record.clear();
          Record.push_back(RECORD_DIAG);
          Record.push_back(getDiagnosticLevel(Kind));
          addLocToRecord(Loc, SM, filename, Record);
          
          
// FIXME: Swift diagnostics currently have no category.
          Record.push_back(0);
          // FIXME: Swift diagnostics currently have no flags.

SourceFile::getFilename & getLineAndColumn

github.com

apple/swift/blob/6293546/lib/SILGen/SILGenProfiling.cpp#L706


      
              return llvm::GlobalValue::PrivateLinkage;
            }
          
          
  llvm_unreachable("Unhandled FormalLinkage in switch.");
          }
          
          
void SILGenProfiling::assignRegionCounters(Decl *Root) {
            const auto &SM = SGM.M.getASTContext().SourceMgr;
          
          
  if (auto *ParentFile = cast<DeclContext>(Root)->getParentSourceFile())
              CurrentFileName = ParentFile->getFilename();
          
          
  MapRegionCounters Mapper(RegionCounterMap);
          
          
  if (auto *AFD = dyn_cast<AbstractFunctionDecl>(Root)) {
              CurrentFuncName = SILDeclRef(AFD).mangle();
              CurrentFuncLinkage = getDeclLinkage(AFD);
            } else if (auto *TLCD = dyn_cast<TopLevelCodeDecl>(Root)) {
              llvm::raw_string_ostream OS{CurrentFuncName};
              OS << "__tlcd_";
              TLCD->getStartLoc().printLineAndColumn(OS, SM);

github.com

apple/swift/blob/6293546/lib/SILGen/SILGenProfiling.cpp#L485-L486


      
          auto Start = SM.getLineAndColumn(Region.getStartLoc());
          auto End = SM.getLineAndColumn(Region.getEndLoc());

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

jrose · 2017-03-16T22:20:25.000Z

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> 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 [SILGen] Make #file to respect #sourceLocation by rintaro · Pull Request #5425 · apple/swift · GitHub
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

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#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#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
https://lists.swift.org/mailman/listinfo/swift-dev

blangmuir · 2017-03-16T22:37:57.000Z

+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 [SILGen] Make #file to respect #sourceLocation by rintaro · Pull Request #5425 · apple/swift · GitHub
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

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#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#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

rintaro · 2017-03-20T15:29:45.000Z

Thanks!

I'd go with "presumed".

I've just started a WIP pull request.

github.com/apple/swift

[WIP][SourceManager] Reorganize methods for line and column

apple:master ← rintaro:srcmanager-linecolumn

opened 03:28PM - 20 Mar 17 UTC

rintaro

+151 -149

Reorganize `SourceManager` methods regarding filename, line and column. https:/…/lists.swift.org/pipermail/swift-dev/Week-of-Mon-20170313/004232.html The first commit is strictly NFC. **Presumed locations ( respects `#sourceLocation` directive)** `getBufferIdentifierForLoc(Loc)` → `getPresumedFilenameForLoc(Loc)` `getLineAndColumn(Loc)` → `getPresumedLineAndColumnForLoc(Loc)` **Physical locations (ignores `#sourceLocation` directive)** `getIdentifierForBuffer(BufferID)` → ASIS new `getIdentifierForBuffer(Loc)` `getLineNumber(Loc)` → `getLineAndColumnInBuffer(Loc).first` `resolveFromLineCol(BufferID, Line, Column)` → `getOffsetForLineAndColumnInBuffer(BufferID, Line, Column)` `getLocForLineCol(BufferID, Line, Column)` → `getLocForLineAndColumnInBuffer(BufferID, Line, Column)` --- ### Audit checklist * AST * [ ] `ASTScope::print` * [ ] `ConformanceLookupTable::compareProtocolConformances` * [ ] `DeclContext::printContext` * [ ] `GenericSignatureBuilder::RequirementSource::print` * [x] `SingleRawComment::SingleRawComment` → Use physical line and column, modified * Basic * [ ] `SourceLoc::printLineAndColumn` * [x] `swift::writeEditsInJson` → Use physical filename, asis * Frontend * [ ] `DiagnosticVerifier::verifyFile` * [x] `SourceManager::GetMessage` → Use presumed filename, line and column, asis. * IDE * [ ] `SerializedDiagnosticConsumer::addLocToRecord` * [ ] `CommentToXMLConverter::visitDocComment` * [ ] `swift::ide::getLocationInfo` * [ ] `swift::ide::reformat` (lib/IDE/Formatting.cpp) * IRGen * [ ] `PrettySourceFileEmission::print` * [ ] `IRGenDebugInfo::IRGenDebugInfo` * Index * [ ] `index::indexSourceFile` * Parse * [x] `Lexer::getLocForStartOfToken` → Unused function, removed * [x] `Parser::parseLineDirective` → Use physical line, asis * [x] `Parser::parseTrailingClosure` → Use physical line, asis * [x] `Parser::parseStmtReturn` → Use physical column, modified * SIL * [ ] `SILLocation::decode` * SILGen * [x] `SILGenFunction::emitLiteral` (`#file` value) Use presumed filename, asis * [x] `SILGenFunction::emitPreconditionOptionalHasValue` Use presumed filename and line, modified * [x] `RValueEmitter::visitMagicIdentifierLiteralExpr` (`#line` and `#column` value) Use presumed line and column, asis * [ ] `CoverageMapping::emitSourceRegions` * Sema * [ ] `NameBinder::addImport` * [x] `FailureDiagnosis::visitApplyExpr` → Use physical line, asis * [x] `swift::performStmtDiagnostics` → Use physical line and column, modified * [ ] `swift::performPCMacro` * [ ] `swift::performPlaygroundTransform` * [x] `static diagnoseAndMigrateVarParameterToBody` → Use physical line, asis * Serialization * [ ] `GroupNameCollectorFromJson::getGroupNameInternal` * SourceKit * [ ] `EditorDiagConsumer::handleDiagnostic` * [ ] `SwiftEditorSyntaxWalker::walkToNodePre` * [ ] `SwiftEditorDocument::replaceText` * swift-ide-test * [ ] `AnnotationPrinter::printLoc` * [ ] `ASTTypePrinter::walkToExprPre` * [ ] `ASTCommentPrinter::walkToDeclPre` * [ ] `USRPrinter::printLoc`

···

2017-03-17 7:37 GMT+09:00 Ben Langmuir <blangmuir@apple.com>:

+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> 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 Apple · GitHub
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

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#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#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
https://lists.swift.org/mailman/listinfo/swift-dev

_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev