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

Symbol protection #139

Merged
merged 2 commits into from
May 4, 2019
Merged

Conversation

WebFreak001
Copy link
Member

Moves protection into DSymbol

Needed for dlang-community/DCD#598

@@ -215,6 +216,11 @@ struct Scope
return getSymbolsByName(name);
}

bool hasSymbolRecursive(const(DSymbol)* symbol) const
Copy link
Member Author

Choose a reason for hiding this comment

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

this is not strictly part of the protection changes, I added it for the use-case of after checking protection in DCD

@WebFreak001
Copy link
Member Author

I don't think this is a very complex change, it just makes the protection available inside a DSymbol. Memory usage per DSymbol would maybe rise a little bit (or well, freeing SemanticSymbol won't free the few protection bytes anymore) but I don't think that would be that big of an issue? Before it was in SemanticSymbol. It was also the only property in there that was only related to the DSymbol instance, so I think it even makes sense from a semantic view to move it into DSymbol as it describes it.

@Basile-z what do you think?

@ghost
Copy link

ghost commented May 2, 2019

I'm struggling on what the other IDEs do. Lazarus do show private members. I don't remember what Delphi did, and otherwise I don't know. I think that this change was proposed by @CyberShadow initially. In other words, I wont be a decision maker here.

@WebFreak001
Copy link
Member Author

for the dsymbol PR here I don't think any of that matters, it simply adds information about the visibility level of a symbol. I think this should be merged and a decision on hiding private members should be made on the DCD PR.

@ghost ghost added the auto-merge-squash label May 4, 2019
@dlang-bot dlang-bot merged commit 5825b71 into dlang-community:master May 4, 2019
@ghost
Copy link

ghost commented May 4, 2019

tagged v0.7.0 since this modifies the API

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants