Skip to content
New issue

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

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

Already on GitHub? # to your account

Add *Machine.cc files that are included by Machine.cc. #338

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

grebe
Copy link
Contributor

@grebe grebe commented Jul 23, 2024

OpenRoad includes different Machine*.cc files on different platforms, and only Linux was previously included. This change makes all files available to be included.

OpenRoad includes different *Machine.cc files on different platforms,
and only Linux was previously included. This change makes all files
available to be included.
@mikesinouye
Copy link
Collaborator

/gcbrun

@QuantamHD
Copy link
Collaborator

This is going to lead to ODR violations. You'll need to bazel select them based on the @platforms// target.

@grebe
Copy link
Contributor Author

grebe commented Jul 23, 2024

I don't think it should because it's stuffed into hdrs. Confusingly, it looks like they're #includeing .cc files.

I don't disagree that selecting them would be less confusing, though I'm a little unsure of how to do that in a way that won't be fighting the platform detection they do.

@QuantamHD
Copy link
Collaborator

I think technically these should be included in the textual_hdrs, and not the regular hdrs since they aren't meant to be compiled.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants