Skip to content

lib/sourcelocation.h: add missing <cstdint> include#4474

Merged
firewave merged 1 commit intocppcheck-opensource:mainfrom
trofi:gcc-13-fix
Sep 16, 2022
Merged

lib/sourcelocation.h: add missing <cstdint> include#4474
firewave merged 1 commit intocppcheck-opensource:mainfrom
trofi:gcc-13-fix

Conversation

@trofi
Copy link
Copy Markdown
Contributor

@trofi trofi commented Sep 16, 2022

Without the change build on upcoming gcc-13 fails as:

In file included from lib/symboldatabase.h:28,
                 from lib/astutils.h:36,
                 from test/testastutils.cpp:20:
lib/sourcelocation.h:52:10: error: 'uint_least32_t' in namespace 'std' does not name a type
   52 |     std::uint_least32_t m_line = 0;
      |          ^~~~~~~~~~~~~~

@firewave
Copy link
Copy Markdown
Collaborator

Thanks for your contribution.

Strange. I just did a run of include-what-you-use and that didn't come up. I just tested it with a reduced case and it is being reported. Something to look into.

We should only include it when needed. But that can be adjusted in later changes.

@trofi
Copy link
Copy Markdown
Contributor Author

trofi commented Sep 16, 2022

I'm not sure how include-what-you-use tracks indirect includes for type declarations. AFAIU uint_least32_t should come out of <cstdint> (or it's wrapper). I did not notice any header wrappers in cppcpeck, but I did not look too closely.

Why it came up only now: in gcc before 13 <string> happened to include <cstdint> for it's own internals. But after a cleanup gcc-13 removed the include. This exposed missing header include in a number of projects including cppcheck. https://gcc.gnu.org/gcc-13/porting_to.html mentions header changes as well.

@trofi
Copy link
Copy Markdown
Contributor Author

trofi commented Sep 16, 2022

Oh, I probably misumderstood your comment about "when needed". I'll move the header under:

#else
struct SourceLocation {
    static SourceLocation current() {

Without the change build on upcoming gcc-13 fails as:

    In file included from lib/symboldatabase.h:28,
                     from lib/astutils.h:36,
                     from test/testastutils.cpp:20:
    lib/sourcelocation.h:52:10: error: 'uint_least32_t' in namespace 'std' does not name a type
       52 |     std::uint_least32_t m_line = 0;
          |          ^~~~~~~~~~~~~~
@trofi
Copy link
Copy Markdown
Contributor Author

trofi commented Sep 16, 2022

Moved #include into #ifdef branch that uses the types directly.

@firewave
Copy link
Copy Markdown
Collaborator

Moved #include into #ifdef branch that uses the types directly.

Yeah - that's what I meant. Thanks!

I'm not sure how include-what-you-use tracks indirect includes for type declarations. AFAIU uint_least32_t should come out of <cstdint> (or it's wrapper). I did not notice any header wrappers in cppcpeck, but I did not look too closely.

It's a limitation of include-what-you-use: include-what-you-use/include-what-you-use#1035.

Fortunately just recently CMake added a feature which allows us to handle it.

Why it came up only now: in gcc before 13 <string> happened to include <cstdint> for it's own internals. But after a cleanup gcc-13 removed the include. This exposed missing header include in a number of projects including cppcheck. https://gcc.gnu.org/gcc-13/porting_to.html mentions header changes as well.

We came across this multiple times in the past which is why older versions of Cppcheck cannot be built with the latest toolchains. But we no longer rely on transitive includes (apparently we still do) so it should be better now.

@firewave firewave merged commit 3b840e7 into cppcheck-opensource:main Sep 16, 2022
@trofi trofi deleted the gcc-13-fix branch September 16, 2022 20:29
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