Skip to content

Conversation

@calebzulawski
Copy link

Based on #436, but contains the full MSVC implementation.

@armandomontanez
Copy link
Collaborator

Wow, this is AWESOME! I'd like to help get this landed in the next week. At first glance this looks pretty good, need to spend a bit more time to look through the rest.

@cerisier
Copy link
Contributor

We're also very interested as part of https://github.com/cerisier/toolchains_llvm_bootstrapped !

Copy link
Collaborator

@armandomontanez armandomontanez left a comment

Choose a reason for hiding this comment

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

Sorry for the radio silence. This is awesome work. Posting my initial concern, but I need to do a bit of a deeper dive to make sure the foundational bits are double-checked.

@@ -0,0 +1,33 @@
load("//cc/toolchains:feature.bzl", "cc_feature")

package(default_visibility = ["//visibility:public"])
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is one piece that jumped out at me. Hyrum's law will likely immediately take effect and people will start depending on these. I think something roughly like this should exist, but I don't want to slip in subtly as part of a larger change.

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