Skip to content

Generate CodeView by deafult for Windows #22625

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

Closed
wants to merge 1 commit into from
Closed

Generate CodeView by deafult for Windows #22625

wants to merge 1 commit into from

Conversation

lanza
Copy link
Contributor

@lanza lanza commented Feb 14, 2019

llvm, lldb and swift combine to offer better support for PDBs on
Windows than it does DWARF. Default to generating PDBs on Windows.

@lanza lanza requested a review from compnerd February 14, 2019 22:51
@lanza
Copy link
Contributor Author

lanza commented Feb 14, 2019

After #22624 goes in this would be the most sane default.

@compnerd
Copy link
Member

Please add a driver level test to ensure that we do this.

@compnerd
Copy link
Member

CC: @dcci

llvm, lldb and swift combine to offer better support for PDBs on
Windows than it does DWARF. Default to generating PDBs on Windows.
@lanza
Copy link
Contributor Author

lanza commented Mar 12, 2019

@compnerd This fails to build on Windows due to the pdb gen bug you hit. WIP

@CodaFi
Copy link
Contributor

CodaFi commented Nov 18, 2019

Let's see what happens

@swift-ci please smoke test

@CodaFi
Copy link
Contributor

CodaFi commented Nov 18, 2019

@swift-ci please test Windows platform

@compnerd
Copy link
Member

compnerd commented Dec 4, 2019

welp, thats going to destroy the test suite on Windows; I think that we should ensure that those work before merging this

Copy link
Member

@dcci dcci left a comment

Choose a reason for hiding this comment

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

Agree. Also it feels like a bug to me that Windows failing doesn't prevent the PR from being merged.

@CodaFi
Copy link
Contributor

CodaFi commented Dec 5, 2019

@lanza I think we can merge this if you XFAIL or fix these tests:

    Swift(windows-x86_64) :: IRGen/condfail_message.swift
    Swift(windows-x86_64) :: DebugInfo/variables.swift
    Swift(windows-x86_64) :: DebugInfo/tuple.swift
    Swift(windows-x86_64) :: DebugInfo/top_level_code.swift
    Swift(windows-x86_64) :: DebugInfo/shadowcopy-linetable.swift
    Swift(windows-x86_64) :: DebugInfo/scopes.swift
    Swift(windows-x86_64) :: DebugInfo/return.swift
    Swift(windows-x86_64) :: DebugInfo/prologue.swift
    Swift(windows-x86_64) :: DebugInfo/patternmatching.swift
    Swift(windows-x86_64) :: DebugInfo/linetable.swift
    Swift(windows-x86_64) :: DebugInfo/line-directive.swift
    Swift(windows-x86_64) :: DebugInfo/letstring.swift
    Swift(windows-x86_64) :: DebugInfo/iteration.swift
    Swift(windows-x86_64) :: DebugInfo/inlinedAt.swift
    Swift(windows-x86_64) :: DebugInfo/inlinescopes.swift
    Swift(windows-x86_64) :: DebugInfo/implicitreturn.swift
    Swift(windows-x86_64) :: DebugInfo/gsil.swift
    Swift(windows-x86_64) :: DebugInfo/foreach.swift
    Swift(windows-x86_64) :: DebugInfo/fnptr.swift
    Swift(windows-x86_64) :: DebugInfo/enum.swift
    Swift(windows-x86_64) :: DebugInfo/columns.swift
    Swift(windows-x86_64) :: DebugInfo/closure-args.swift
    Swift(windows-x86_64) :: DebugInfo/basic.swift
    Swift(windows-x86_64) :: DebugInfo/bool.swift
    Swift(windows-x86_64) :: DebugInfo/any.swift
    Swift(windows-x86_64) :: DebugInfo/alloca-init.swift
    Swift(windows-x86_64) :: DebugInfo/NestedTypes.swift
    Swift(windows-x86_64) :: DebugInfo/Imports.swift
    Swift(windows-x86_64) :: DebugInfo/ImportsStdlib.swift
    Swift(windows-x86_64) :: DebugInfo/EagerTypeMetadata.swift

@dcci
Copy link
Member

dcci commented Dec 5, 2019

I wouldn't really XFAIL these tests. It means that there are potentially big holes in CodeView Windows support and merging this patch would lead to false expectations of what works and what doesn't (or maybe tests just need to be updated and everything works fine, in which case, well, we should do it sooner rather than later).

@CodaFi
Copy link
Contributor

CodaFi commented Apr 14, 2020

@lanza I'm going to close this out since it seems like this needs to be handled incrementally to avoid disrupting the Windows build.

@CodaFi CodaFi closed this Apr 14, 2020
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.

4 participants