Skip to content

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

Merged
cshung merged 1 commit into
dotnet:mainfrom
cshung:experiment-only
Aug 31, 2021
Merged

Eliminate DACCESS_COMPILE from gc.cpp and gcpriv.h#57607
cshung merged 1 commit into
dotnet:mainfrom
cshung:experiment-only

Conversation

@cshung

@cshung cshung commented Aug 17, 2021

Copy link
Copy Markdown
Contributor

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 ghost added the area-GC-coreclr label Aug 17, 2021
@ghost

ghost commented Aug 17, 2021

Copy link
Copy Markdown

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

@PeterSolMS PeterSolMS left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@PeterSolMS

Copy link
Copy Markdown
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

cshung commented Aug 31, 2021

Copy link
Copy Markdown
Contributor Author

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

cshung commented Aug 31, 2021

Copy link
Copy Markdown
Contributor Author

#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