Skip to content

[windows] Adapt test to pass on Windows. #27823

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
Oct 29, 2019

Conversation

drodriguez
Copy link
Contributor

The underscores weren't printed while compiling for Windows between the
symbolic and the mangled name tokens. Make the underscores optional, to
allow the test pass on Windows.

This should fix the failing test on Windows.

I am not sure this is the right fix. I am just uploading in order to find out the best way. I don't understand what those underscores mean, but since they were added in 3ca0859, I imagine they are important. I reviewed the code of #27666, and I don't obviously see where the difference in Windows might come from or what those underscores actually mean. Any alternative solution would be greatly appreciated.

The underscores weren't printed while compiling for Windows between the
symbolic and the mangled name tokens. Make the underscores optional, to
allow the test pass on Windows.
@drodriguez
Copy link
Contributor Author

(this might fail with the Error 87 I am trying to figure out, but reading the logs will let me know if the test also passes in CI or not)

@swift-ci please test Windows platform

@drodriguez
Copy link
Contributor Author

No failure in the test themselves, still seeing the unexplained WindowsError: [Error 87] The parameter is incorrect.

@swift-ci please smoke test

@jckarter
Copy link
Contributor

If you're feeling more ambitious, it'd be great to implement PE support for loading relocated pointers from the import table to ObjectMemoryReader in swift-reflection-dump, which would eliminate the root cause of the divergence here.

@drodriguez
Copy link
Contributor Author

I am going to merge this version for the time being, in order to have a green on Windows again. I will try to talk with @alexshap later and try to figure out the alternative approach.

@drodriguez drodriguez merged commit 8e41df8 into swiftlang:master Oct 29, 2019
@drodriguez drodriguez deleted the windows-fix-text-ir-output branch October 29, 2019 16:49
@jckarter
Copy link
Contributor

SGTM, you should definitely commit this to keep the builds green, I was just suggesting a good follow-up Windows quality-of-implementation task.

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.

2 participants