Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Enable CodeView debug information with MSVC on Windows #2334

Merged
merged 2 commits into from
Nov 19, 2017
Merged

Enable CodeView debug information with MSVC on Windows #2334

merged 2 commits into from
Nov 19, 2017

Conversation

chalcolith
Copy link
Member

@chalcolith chalcolith commented Nov 13, 2017 •
edited
Loading

@oraoto mentioned in #2331 that debug information is not output for Pony programs on Windows.
This PR implements CodeView debug information so that you can see functions and variables when debugging in Windows.

@chalcolith chalcolith added changelog - added Automatically add "Added" CHANGELOG entry on merge do not merge This PR should not be merged at this time labels Nov 13, 2017
@chalcolith
Enable CodeView debug information with MSVC on Windows
f329da1 
@oraoto mentioned in ponylang#2331 that debug information is not output for Pony programs on Windows.
I have implemented his fix, and it seems to work for me, unlike him.

Let's see what happens in CI.
@chalcolith
CodeView debug information for Windows does not like "non-class" meth…
842e09d 
…ods.

Following [Rust's solution](rust-lang/rust#35991 (comment)), we make a dummy scope for primitive methods.

This change also reorders some imports in codegen.h and gendebug.cc, as gendebug.h now depends on having PONY_LLVM defined.
@chalcolith chalcolith removed the do not merge This PR should not be merged at this time label Nov 19, 2017
@chalcolith
Copy link
Member Author

It seems that Rust encountered the same thing when enabling CodeView last year.

CodeView output doesn't like methods on non-class-like things. My latest commit fixes this.

@jemc jemc merged commit 5efe0e0 into ponylang:master Nov 19, 2017
ponylang-main added a commit that referenced this pull request Nov 19, 2017
@ponylang-main
Update CHANGELOG for PR #2334 [skip ci]
a8a1323
@krig
Copy link
Contributor

krig commented Nov 19, 2017

Just a note for this commit together with the llvm500 branch: The API of DIBuilder::createNameSpace changed in LLVM 4, adding an extra boolean parameter ExportSymbols. Passing false as the value seems fine.

I'm not entirely sure about this, but it seems like it changes again in LLVM 5 removing the file and line parameters...

krig added a commit to krig/ponyc that referenced this pull request Nov 19, 2017
@krig
Pass ExportSymbols argument to DIBuilder::createNameSpace with LLVM >…
0197a11 
…=4.0 (ponylang#2334)

In LLVM 4.0, an extra parameter ExportSymbols was added to
DIBuilder::createNameSpace.
@chalcolith
Copy link
Member Author

chalcolith commented Nov 19, 2017 •
edited
Loading

That's why I made the C wrapper in gendebug.h conditional on PONY_LLVM.

@chalcolith chalcolith mentioned this pull request Nov 19, 2017
Closed
@krig
Copy link
Contributor

krig commented Nov 19, 2017

To be clear, compilation fails. The implementation would need to be wrapped as well in that case..

@chalcolith
Copy link
Member Author

I know, I have not yet updated my llvm500 branch to handle the various changes to that function signature.

@chalcolith chalcolith mentioned this pull request Nov 19, 2017
Merged
@krig
Copy link
Contributor

krig commented Nov 19, 2017

Alright :)

@chalcolith chalcolith deleted the fix_2331 branch November 19, 2017 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
changelog - added Automatically add "Added" CHANGELOG entry on merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chalcolith @krig @jemc