Skip to content

prefer std::u?int[0-9]+_t and include <cstdint> else include <stdint.h>#2302

Merged
alecjacobson merged 1 commit intomainfrom
alecjacobson/std
Oct 14, 2023
Merged

prefer std::u?int[0-9]+_t and include <cstdint> else include <stdint.h>#2302
alecjacobson merged 1 commit intomainfrom
alecjacobson/std

Conversation

@alecjacobson
Copy link
Copy Markdown
Contributor

Fixes this kind of problem https://www.reddit.com/r/Fedora/comments/16issng/fedora_38_cant_run_simple_libigl_code/

#include <cstdint> or #include <stdint.h> is often missing (and accidentally included by some other path) or a global namespace type is used (uint64_t) with the assumption that #include <cstdint> will define it (it doesn't have to). So, prefer to use std:: namespace types and #include <cstdint>. In some cases, where the code was ported from elsewhere I used #include <stdint.h> to match the global namespace types. (also remove some unnecessary size_t types along the way)

@jdumas
Copy link
Copy Markdown
Collaborator

jdumas commented Oct 13, 2023

I don't understand why you need the std:: namespace? Isn't it enough to include <cstdint>? I've never had any issue without the std:: prefix for these types whatsoever.

@alecjacobson
Copy link
Copy Markdown
Contributor Author

It was news to me too. But apparently global namespace is only guaranteed with the c headers

@alecjacobson alecjacobson merged commit 7c9387c into main Oct 14, 2023
@alecjacobson alecjacobson deleted the alecjacobson/std branch October 14, 2023 00:15
@jdumas
Copy link
Copy Markdown
Collaborator

jdumas commented Oct 14, 2023

Yikes... Well I'm choosing to close my eyes until someone complains and I cannot do otherwise but to change my code...

@IslamOmar-360Imaging
Copy link
Copy Markdown

@alecjacobson https://github.com/libigl/libigl/blob/main/include/igl/path_to_executable.cpp#L25 gives error as https://github.com/libigl/libigl/blob/main/include/igl/path_to_executable.cpp#L17 doesn't provide the namespace for uint32_t and you are adding using namespace std, so

I think you should either add <cstdint> or use uint32_t without std::.

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.

3 participants