-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
C++: Support using enum
declarations.
#17006
Conversation
b7710c2
to
d81d2d8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two small questions, otherwise this looks fine to me.
usings(u, target, loc, kind) and | ||
kind != 3 | ||
select u, target, loc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing in the database can refer to a UsingEntry
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in af562f1. (Don't think we need to do anything on the upgrade side.)
from UsingEntry u, Element target, Location loc, int kind | ||
where | ||
usings(u, target, loc) and | ||
if target instanceof @namespace then kind = 2 else kind = 1 | ||
select u, target, loc, kind |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume you did some basic testing to ensure this works as expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I spot checked the version you reviewed by creating a database for the usings test with the before/after CLI and running the queries with after/before CLIs respectively. I'm doing the same for the using_container
fix now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just confirmed all looks good with the additional typo fix in 3defc8b.
d81d2d8
to
af562f1
Compare
Previously, using entries were classified into using-declarations and using-directives based on whether they refer to a namespace or not.
With
using enum
declarations, we can no longer use the target to distinguish using-declarations and using-enum-declarations. E.g. the following:Therefore we add a kind column. For consistency and simplicity, using-directives are also migrated to a separate kind.