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.

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

Fix New Dynarec build failures arising from commit bdb8a88 #95

Merged
merged 1 commit into from
Mar 6, 2015

Conversation

Nebuleon
Copy link
Contributor

@Nebuleon Nebuleon commented Mar 5, 2015

Hopefully fixes #94. I looked in the new subdirectories of new_dynarec and this should fix all inclusion issues.

…lus#94)

This should cover broken inclusions after the move to platform-specific
code subdirectories.
@Narann
Copy link
Member

Narann commented Mar 5, 2015

@Gillou68310, can you confirm this fix #94? :)

@gizmo98
Copy link
Contributor

gizmo98 commented Mar 5, 2015

Checked. new_dynarec arm is working!

@Gillou68310
Copy link
Member

Thanks for taking care of this @Nebuleon ;-)
But I think that cp0_private.h and main.h could actually be removed from assem_arm.c as they are already included in new_dynarec.c.

@Narann
Copy link
Member

Narann commented Mar 5, 2015

Do you think what @Gillou68310 said is relevant @Nebuleon? If so, you can update this PR before I merge it.

@Gillou68310
Copy link
Member

@Narann is it possible to ask travis to build both old and new dynarec so we won't end up with those kind a errors?

@Nebuleon
Copy link
Contributor Author

Nebuleon commented Mar 5, 2015

arm/assem_arm.c and new_dynarec.c are different compilation units; including cp0_private.h in the latter would have no effect in the former. The ARM-specific file requires g_cp0_regs (for example, line 1165 of assem_arm.c as of commit f648835), so it should continue to include cp0_private.h (at the proper path).

wow, including .c files in .c files... I really don't like this, but if assem_PLATFORM.c is never compiled on its own, only as part of new_dynarec.c, that could work. However, it's still good practice to keep the includes required by the included .c file.

@Narann
Copy link
Member

Narann commented Mar 5, 2015

Good idea @Gillou68310!

But I have no idea how to do that. I've created a ticket #96.

@Gillou68310
Copy link
Member

wow, including .c files in .c files... I really don't like this.

Yeah I agree that's not the best practice!

Ok I think we can leave these include files for the moment, but the new dynarec will need a big cleanup at some point.

Good idea @Gillou68310!
But I have no idea how to do that. I've created a ticket #96.

Thanks @Narann I will check this!

Narann added a commit that referenced this pull request Mar 6, 2015
Fix New Dynarec build failures arising from commit bdb8a88
@Narann Narann merged commit 35fc02b into mupen64plus:master Mar 6, 2015
@Nebuleon Nebuleon deleted the nd-build-fix branch March 9, 2015 01:18
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.

new_dynarec arm is broken
4 participants