Skip to content

[DebugInfo] Use Stmt EndLoc if SILLocation passed in is the Stmt EndLoc. #79833

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 19, 2025

Conversation

rastogishubham
Copy link
Contributor

When creating an ExtendedASTNodeLoc from a SILLocation, if the SILLocation passed in belongs to a swift::Stmt, we only ever use the Stmt's StartLoc for the SourceLocation. If the SILLocation passed in, has a SourceLocation that matches the EndLoc of the Stmt, we should correctly set the primary ASTNodeTy PointerIntPair's integer to 1, to denote that the SourceLocation dervied from the Stmt points to the EndLoc.

To explain a little more:

When we emit a hop_to_execution instruction in SIL, we call RegularLocation::getDebugOnlyLocation on the SILLocation being passed in, which calls RegularLocation::getDebugOnlyExtendedASTNodeLoc. This function will create an ExtendedASTNodeLoc which has two AST locations, a primary location and one used for debugging:

struct ExtendedASTNodeLoc : public SILAllocated<ExtendedASTNodeLoc> {
    /// Primary AST location, always used for diagnostics.
    ASTNodeTy primary;
    /// Sometimes the location for diagnostics needs to be different than the
    /// one used to emit the line table for debugging.
    ASTNodeTy forDebugging;
    
    ExtendedASTNodeLoc(ASTNodeTy primary, ASTNodeTy forDebugging) :
      primary(primary), forDebugging(forDebugging) {}
  };

If we look at how a SourceLoc is derived from a SILLocation we can see that the SILLocation::getSourceLoc always calls getStartLoc if the pointer the ASTNodeTy holds is a swift::Stmt*, unless the SILLocation::pointsToEnd function returns true.

The SILLocation::pointsToEnd function checks the primary location to see if it has a 1 as it's PointerIntPair's integer value.

bool SILLocation::pointsToEnd() const {
  switch (getStorageKind()) {
  case ASTNodeKind:
    return storage.ASTNodeLoc.getInt();
  case ExtendedASTNodeKind:
    return storage.extendedASTNodeLoc->primary.getInt();
  default:
    return false;
  }
}

Therefore, if we know that the SILLocation being passed in when creating our ExtendedASTNodeLoc is pointing to the EndLoc of the swift::Stmt, we need to set the primary ASTNodeTy's integer to 1.

@rastogishubham
Copy link
Contributor Author

@swift-ci please smoke test Linux

@rastogishubham
Copy link
Contributor Author

@swift-ci please smoke test macOS

@rastogishubham
Copy link
Contributor Author

@swift-ci please test Windows

@rastogishubham
Copy link
Contributor Author

Please test with following pull request:
swiftlang/llvm-project#10200

@swift-ci Please smoke test Linux

@rastogishubham
Copy link
Contributor Author

Please test with following pull request:
#4574

@swift-ci Please smoke test macOS

@rastogishubham
Copy link
Contributor Author

Please test with following pull request:
#4574

@swift-ci Please test Windows

Copy link
Contributor

@felipepiovezan felipepiovezan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adrian-prantl probably understands this code a bit better, so I'll let him do the honors. I'm not familiar with this integer in the AST node

@rastogishubham
Copy link
Contributor Author

Please test with following pull request:
swiftlang/llvm-project#10200

@swift-ci Please smoke test Linux

@rastogishubham
Copy link
Contributor Author

Please test with following pull request:
swiftlang/llvm-project#10200

@swift-ci Please smoke test macOS

@rastogishubham
Copy link
Contributor Author

Please test with following pull request:
swiftlang/llvm-project#10200

@swift-ci Please test Windows

@rastogishubham
Copy link
Contributor Author

Please test with following pull request:
swiftlang/llvm-project#10200

@swift-ci Please smoke test Linux

@rastogishubham
Copy link
Contributor Author

Please test with following pull request:
swiftlang/llvm-project#10200

@swift-ci Please smoke test macOS

@rastogishubham
Copy link
Contributor Author

Please test with following pull request:
swiftlang/llvm-project#10200

@swift-ci Please test Windows

@rastogishubham
Copy link
Contributor Author

@swift-ci please smoke test macOS

When creating an ExtendedASTNodeLoc from a SILLocation, if the
SILLocation passed in belongs to a swift::Stmt, we only ever use the
Stmt's StartLoc for the SourceLocation. If the SILLocation passed in,
has a SourceLocation that matches the EndLoc of the Stmt, we should
correctly set the primary ASTNodeTy PointerUnion's integer to 1, to
denote that the SourceLocation dervied from the Stmt points to the
EndLoc.
@rastogishubham
Copy link
Contributor Author

Please test with following pull request:
swiftlang/llvm-project#10200

@swift-ci Please smoke test Linux

@rastogishubham
Copy link
Contributor Author

Please test with following pull request:
swiftlang/llvm-project#10200

@swift-ci Please smoke test macOS

@rastogishubham
Copy link
Contributor Author

Please test with following pull request:
swiftlang/llvm-project#10200

@swift-ci Please test Windows

@rastogishubham rastogishubham merged commit 6f09b80 into swiftlang:main Mar 19, 2025
3 checks passed
@rastogishubham rastogishubham deleted the FixHopToExe branch March 19, 2025 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants