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

Fix AVR target to define ints properly #12817

Merged
merged 5 commits into from
Dec 10, 2019
Merged

Fix AVR target to define ints properly #12817

merged 5 commits into from
Dec 10, 2019

Conversation

PMunch
Copy link
Contributor

@PMunch PMunch commented Dec 5, 2019

HAVE_STDINT_H wasn't defined for the AVR compiler so the NU32 integer type was typedef'ed to an unsigned int. On the AVR platform however an unsigned int is 16 bits in size. This would lead to weird behaviour when trying to use these types. This check fixes the issue by defining HAVE_STDINT_H when the AVR compiler sets the __AVR flag.

@Araq
Copy link
Member

Araq commented Dec 5, 2019

Wouldn't it be a better idea to check for the GCC/Clang version that ship stdint.h?

@PMunch
Copy link
Contributor Author

PMunch commented Dec 9, 2019

So it appears that C++ (which I was using to interface with Arduino libraries) doesn't specify __STDC_VERSION__ (not that it would have mattered since it was misspelled as __STD_VERSION__). So the existing check for C99 didn't work for C++. In C++11 however it is recommended to use #include <cstdint> and std::uint32_t instead of the old C way. Turns out however that the avr-gcc compiler doesn't ship with this library. So in my latest commit I have fixed the spelling mistake of __STDC_VERSION__, I've added a check for C++11 and C++98 to use their preferred way, and I've kept the check for __AVR__ since it behaves differently. I've also added #ifdef __INT32_TYPE__ checks to see if the compiler defines these values (which is what stdint.h/cstdint uses at least on GCC to define these types), and define the types directly to those if none of the other methods work. If it doesn't even support that it will fall back to the best guess types like it did before (maybe this should just be an error to avoid similar issues in the future?).

Oh, and I've also re-ordered the listings so it goes int8..int64, uint8..uint64 instead of int8..int32, uint8..uint32, int64, uint64 or whatever the old order was.

@Araq Araq added the merge_when_passes_CI mergeable once green label Dec 9, 2019
@PMunch
Copy link
Contributor Author

PMunch commented Dec 9, 2019

Don't actually merge this yet by the way, I was just testing something

@PMunch
Copy link
Contributor Author

PMunch commented Dec 9, 2019

If it passes now it should be fine to merge

@Araq Araq merged commit 65fd95b into nim-lang:devel Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge_when_passes_CI mergeable once green
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants