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

C++: Support using enum declarations. #17006

Merged
merged 5 commits into from
Jul 23, 2024
Merged

C++: Support using enum declarations. #17006

merged 5 commits into from
Jul 23, 2024

Conversation

sashabu
Copy link
Contributor

@sashabu sashabu commented Jul 17, 2024

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:

namespace foo {
enum class Bar { one, two };
}
using foo::Bar;  // Brings `Bar` into scope
using enum foo::Bar;  // Brings `one` and `two` into scope.
using foo::Bar::one; using foo::Bar::two;  // Brings `one` and `two` into scope with different syntax.

Therefore we add a kind column. For consistency and simplicity, using-directives are also migrated to a separate kind.

@sashabu sashabu added the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label Jul 17, 2024
@github-actions github-actions bot added the C++ label Jul 17, 2024
@sashabu sashabu force-pushed the sashabu/using-enum branch 7 times, most recently from b7710c2 to d81d2d8 Compare July 17, 2024 19:48
@sashabu sashabu marked this pull request as ready for review July 17, 2024 19:52
@sashabu sashabu requested a review from a team as a code owner July 17, 2024 19:52
Copy link
Contributor

@jketema jketema left a 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.

Comment on lines +15 to +17
usings(u, target, loc, kind) and
kind != 3
select u, target, loc
Copy link
Contributor

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?

Copy link
Contributor Author

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.)

Comment on lines +13 to +17
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
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

@sashabu sashabu requested a review from jketema July 22, 2024 17:29
@jketema jketema merged commit d257331 into main Jul 23, 2024
12 of 16 checks passed
@jketema jketema deleted the sashabu/using-enum branch July 23, 2024 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ depends on internal PR This PR should only be merged in sync with an internal Semmle PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants