Skip to content

Fix #13847 (simplifyTypedef should not simplify all using statements) #7527

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

Merged
merged 1 commit into from
May 17, 2025

Conversation

danmar
Copy link
Owner

@danmar danmar commented May 14, 2025

No description provided.

@danmar danmar changed the title Fix #13847 (simplifyTypedef should not simplify ) Fix #13847 (simplifyTypedef should not simplify all using statements) May 14, 2025
@chrchr-github
Copy link
Collaborator

chrchr-github commented May 14, 2025

Do we handle this code?

namespace x {
    struct a{};
}

int main() {
    int a = 0;
    using x::a;
    return a + 2;
}

FYI accepted by clang and gcc, rejected by msvc.

@danmar
Copy link
Owner Author

danmar commented May 14, 2025

Do we handle this code?

I thought it would.. do I understand it correctly that the using x::a should be ignored..

@chrchr-github
Copy link
Collaborator

Do we handle this code?

I thought it would.. do I understand it correctly that the using x::a should be ignored..

Yes, at least two compilers interpret a as a variable.

@danmar
Copy link
Owner Author

danmar commented May 15, 2025

After simplifyTypedef the tokens are now:

1: namespace x {
2: struct a { } ;
3: }
4:
5: int main ( ) {
6: int a ; a = 0 ;
7: using x :: a ;
8: return a + 2 ;
9: }

And then after this the simplifyUsing is called which transforms the code to:

1: namespace x {
2: struct a { } ;
3: }
4:
5: int main ( ) {
6: int a ; a = 0 ;
7:
8: return x :: a + 2 ;
9: }

@danmar
Copy link
Owner Author

danmar commented May 15, 2025

I feel I would rather not update simplifyUsing also in this PR.

But if I move the variable a little then clang suddenly seems to reject the code:

namespace x {
    struct a{};
}

int main() {
    int a = 0;
    {
      using x::a;
      return a + 2;
    }
}

so it seems if we make some exception in simplifyUsing it should only be for the variables in local scope

@danmar danmar requested a review from Copilot May 16, 2025 11:29
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses issue #13847 by preventing the simplifyTypedef pass from converting all using statements into typedefs in C++ code.

  • Added a new tokenizer test (tokenize41) to cover the no-simplify-using scenario.
  • Introduced typedefInfo3 to verify that dumpTypedefInfo ignores using statements.
  • Removed the old simplifyUsingToTypedef method and inlined minimal usingtypedef logic into simplifyTypedefCpp.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
test/testtokenize.cpp Added tokenize41 test for using statements (#13847).
test/testsimplifytypedef.cpp Registered new typedefInfo3 test; removed typedefInfo2 registration.
lib/tokenize.h Dropped simplifyUsingToTypedef declaration.
lib/tokenize.cpp Removed simplifyUsingToTypedef implementation; inlined simple usingtypedef pattern.
Comments suppressed due to low confidence (3)

test/testtokenize.cpp:873

  • The test currently discards errout_str(). Add an assertion like ASSERT_EQUALS("", errout_str()); to ensure no unexpected errors are emitted during tokenization.
(void)errout_str();

test/testsimplifytypedef.cpp:253

  • The original TEST_CASE(typedefInfo2) registration was removed, so its test will no longer run. Either restore TEST_CASE(typedefInfo2) or remove its function definition to keep the suite consistent.
TEST_CASE(typedefInfo3);

lib/tokenize.cpp:1951

  • This pattern only matches single-level using X::Y; and misses global-namespace or nested qualifiers (e.g., using ::X::Y;). Consider extending the match to also cover using :: %name% ::|; or refactor using logic to handle all valid cases.
if (cpp && Token::Match(tok2->previous(), "using %name% ::|;")) {

@danmar
Copy link
Owner Author

danmar commented May 17, 2025

This PR is about making simplifyTypedef less aggressive. It would simplify "using" that was not supposed to be simplified.

@danmar danmar merged commit e4b51d7 into danmar:main May 17, 2025
65 of 66 checks passed
@danmar
Copy link
Owner Author

danmar commented May 17, 2025

I created https://trac.cppcheck.net/ticket/13872 about your issue

@danmar danmar deleted the fix-13847 branch May 17, 2025 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants