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

Eliminate DACCESS_COMPILE from gc.cpp and gcpriv.h #57607

Merged
merged 1 commit into from
Aug 31, 2021

Conversation

cshung
Copy link
Member

@cshung cshung commented Aug 17, 2021

It appears to me that DACCESS_COMPILE is never defined on gc.cpp and gcpriv.h. Eliminate them from the files to simplify things.

@ghost
Copy link

ghost commented Aug 17, 2021

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

Do not merge - this is an experiment only

Author: cshung
Assignees: -
Labels:

area-GC-coreclr

Milestone: -

@cshung cshung changed the title Experiment only Eliminate DACCESS_COMPILE from gc.cpp and gcpriv.h Aug 19, 2021
@cshung cshung requested a review from PeterSolMS August 20, 2021 02:08
Copy link
Contributor

@PeterSolMS PeterSolMS left a comment

Choose a reason for hiding this comment

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

LGTM!

@PeterSolMS
Copy link
Contributor

Looks good to me.

Perhaps we should protect against DACCESS_COMPILE being defined after all through some source code or build change though?

Something like:

#ifdef DACCESS_COMPILE
#error this source file should not be compiled with DACCESS_COMPILE!
#endif //DACCESS_COMPILE

@cshung
Copy link
Member Author

cshung commented Aug 31, 2021

Looks good to me.

Perhaps we should protect against DACCESS_COMPILE being defined after all through some source code or build change though?

Something like:

#ifdef DACCESS_COMPILE
#error this source file should not be compiled with DACCESS_COMPILE!
#endif //DACCESS_COMPILE

I will add this in my next PR.

@cshung cshung merged commit 8b94b58 into dotnet:main Aug 31, 2021
@cshung cshung deleted the experiment-only branch August 31, 2021 16:58
@cshung
Copy link
Member Author

cshung commented Aug 31, 2021

#58120

@ghost ghost locked as resolved and limited conversation to collaborators Sep 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants