avr-gcc Feature: More Warnings

Go To Last Post
39 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

F'up of avr-gcc: New Features?

ChaunceyGardiner wrote:
I would like to see a lot more warnings. We get away with way too much.

The simplest example I can think of is

int i;
char c = i;

This feature is already there, but you have to use it:

    :2:10: warning: conversion to 'char' from 'int' may alter its value [-Wconversion]
This code is not valid in C. Wrapping the code into a function, you'll get
    foo.c: In function 'f': :2:1: warning: conversion to 'char' from 'int' may alter its value [-Wconversion]
    char c = i;
    ^
    :2:6: warning: unused variable 'c' [-Wunused-variable]
    char c = i;
    ^
    :2:6: warning: 'i' is used uninitialized in this function [-Wuninitialized]
If you want to learn more aboud diagnostics, read GCC: Options to Request or Suppress Warnings and / or try
    avr-gcc --help=warnings
There are even more warnings, cf.

"¢ Options Controlling C++ Dialect
"¢ C and Objective-C-only Warning Options
"¢ Options for Debugging Your Program or GCC
"¢ ...

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

In that case, I would like to see that and other elementary warnings included in -Wall (at least).

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Also, I would like to see that warning relaxed when it comes to assigning integer expressions to 8-bit variables.

For instance,

ADCSRA &= ~(1 << ADEN);

generates that warning. That is completely useless and unnecessary.

I want warnings when I assign something that is too big to fit, unless I explicitly make it fit with a cast. I do not want to add casts in cases like the one above, and the warning is just noise in that situation.

That sort of noise renders the warning useless for me.

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I don't understand you point. The right side of the operation is obviously sizeof(int), the size of the target is 1. How is the compiler supposed to guess what's comfortable for you and what's not?

If you really care about such situations, use

ADCSRA &= (uint8_t) ~(1 << ADEN);

or similar.

It's not uncommon that with a new warning turned on, you have to fix the places where they pop up: Either silence the warning if it is not harmful, or fix the potentially wrong code.

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Why on Earth would I want to add that cast ? The compiler should see that the result of manipulating that constant at compile time is small enough to fit in 8 bits.

I want to be warned if I do something that may cause a problem. The above mentioned example code is never going to cause a problem.

After you told me that this warning exists, I enabled it for one of my smaller projects. That resulted in 59 useless warnings about Atmel compile-time constants that fit in 8 bits. If there was a problem in my code, it would drown in the noise.

Obfuscating my source code with unnecessary casts is not an option.

The fact that you don't see it - even when you ask what the community wants - just adds to the frustration. The lack of useful warnings in avrgcc is a disgrace.

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You should read C-standard about integer promotion...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Along similar lines, this code does not generate a warning:

char x = (1 << 7);

However, this does:

char x = ~(1 << 7);

It's ridiculous.

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

demiurg_spb wrote:
You should read C-standard about integer promotion...
You should engage your common sense.

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The constant will /never/ fit in 8 bits, it's something like

(int) 0b1111111111011111

that you are operating with and writing to an uint8_t.

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The lack of common sense is sticking out like a sore thumb.

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

> The lack of common sense is sticking out like a sore thumb.

Shut up with that, please.

Compilers are stupid machines, which cannot employ something like
"common sense". If someone requests a 16-bit integer constant
by writing "~(1 << 7)", how on earth should the compiler notice
that the developer is actually not interested in its upper 8 bits?

If you don't like the way the C standard is written, then don't
use this language. Or, don't enable that warning.

Jörg Wunsch

Please don't send me PMs, use email if you want to approach me personally.

Last Edited: Mon. Jan 21, 2013 - 05:01 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I've always thought that ALL warnings should be enabled by default - they don't break compilation unless you request it, so having warnings crop up in old code doesn't hurt anything and may help make code better. If a particular warning is unwanted, it should be turned *off* rather than having to be turned *on*.

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

> I've always thought that ALL warnings should be enabled by default

Then, just add -Wextra to the default set of warning options in Atmel
Studio ... Oh, while you are at it, please do also:

  • remove the option to generate RJMP/RCALL instead of JMP/CALL alltogether; it's very dangerous to generate code that won't link;
    the correct way for this would be to add -mrelax to the linker's
    commandline (which is essentially a mandatory option for all
    devices larger than 128 KiB)

  • completely remove the option -fpack-struct; it's a pointless option in AVR-GCC, as the AVR doesn't feature structure padding
    anyway

  • disable -fshort-enums by default; it risks being API incompatible when linking with code that has not been compiled that way; the
    better way is to use __attribute__((packed)) on the respective
    enum declaration consistently throughout the project (header
    and source files)

Jörg Wunsch

Please don't send me PMs, use email if you want to approach me personally.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Good ideas Joerg - can you please file them in the AS6 tracker so that the tools team can see it?

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

@Dean:

Notice that

    -fpack-struct -fshort-enums
    -funsinged-bitfields
    -funsigned-char
are not optimization options! These are options that change the binary interface (ABI)!

-mshort-calls is deprecated since 4.7 and has been removed in 4.8, cf. the release notes.

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
If a particular warning is unwanted, it should be turned *off* rather than having to be turned *on*.

Does gcc have the ability to disable/enable particular warnings? That can be sort-of handy.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

westfw wrote:
Quote:
If a particular warning is unwanted, it should be turned *off* rather than having to be turned *on*.

Does gcc have the ability to disable/enable particular warnings? That can be sort-of handy.

Refer to the link of the first post

SprinterSB wrote:

If you want to learn more aboud diagnostics, read GCC: Options to Request or Suppress Warnings

Alex

"For every effect there is a root cause. Find and address the root cause rather than try to fix the effect, as there is no end to the latter."
Author Unknown

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Does gcc have the ability to disable/enable particular warnings?

Err yes. What's more in the avr-gcc version in AS6 all the warnings you get are suffixed with details of which warning created them so you know which one to choose. As Alex notes SprinteSB gave a link to the user manual:
gcc manual wrote:
Each of these specific warning options also has a negative form beginning `-Wno-' to turn off warnings;

So -Wno-conversion etc.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ah; I couldn't tell whether those were particular errors, or just classes of errors. (actually, I still can't. Intel's "icc" compiler has -Wnnnn to turn of warning number nnnn, with each warning/error assigned a number. nnnn can really be 4 digits, so it looks like this is finer control than provided by gcc.)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

dl8dtl wrote:

Compilers are stupid machines, which cannot employ something like
"common sense". If someone requests a 16-bit integer constant
by writing "~(1 << 7)", how on earth should the compiler notice
that the developer is actually not interested in its upper 8 bits?

I run into this stuff in C# a lot, but worse, the result of most operations are promoted to an int, and you end up with casts everywhere. Yet some

Real solution would be a attribute applied to 'variables' like PORTB that applies an implicit cast to the set operation.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It's not rocket science. If I write

x &= ~(1 << 2);

how can there be any doubt that I want to clear bit 2 in x and do nothing else (given that sizeof x <= sizeof(int)) ?

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Another way to say it:

x &= ~(1 << 2);

is the same as

x = x & ~(1 << 2);

The RHS is

x & ~(1 << 2);

and while that is 16 bits, it's only going to have 8 significant bits if x is an unsigned 8-bit variable. The upper 8 bits will be zero. Hence, the result will fit in x without information being lost.

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

How about

uint8_t x;
x &= -5;

or

x &= 0xFFFB;

or

x &= 3*'B'-4*'A'+071;

then?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

> x = x & ~(1 << 2);

To the compiler, it is:

x = (int)x & 0xfffb;

due to the rules the C standard requires.

Jörg Wunsch

Please don't send me PMs, use email if you want to approach me personally.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

snigelen wrote:
How about
uint8_t x;
x &= -5;

or

x &= 0xFFFB;

or

x &= 3*'B'-4*'A'+071;

then?


Yeah, what about it ? What is your intention, when you write code like that ?

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

dl8dtl wrote:
> x = x & ~(1 << 2);

To the compiler, it is:

x = (int)x & 0xfffb;

due to the rules the C standard requires.


Yes, and the number of significant bits is going to depend on the size of x. That is also a part of the C standard.

Along similar lines,

x = 1;

is also

x = (int) 1;

to the compiler. How come we don't get a warning there ?

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ChaunceyGardiner wrote:
Yeah, what about it ? What is your intention, when you write code like that ?

The compiler don't (and shouldn't) care about how I write my constant expression. They are all equivalent to "x &= ~(1<<2);". Do you really think the compiler should behave different if I happen to use a shift operator in a constant expression?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

snigelen,

Yes, I think the compiler should take into consideration how I use constants. Hopefully, the shift operator is not a part of this problem, it seems more likely that the bitwise not is the culprit.

Either way, you have not shown any realistic case where a warning for something like this is a good thing.

The net result of thinking like you do is that hardly anybody is going to enable that warning.

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ChaunceyGardiner wrote:
snigelen,

Yes, I think the compiler should take into consideration how I use constants.

Then you can't use C.
Quote:
Either way, you have not shown any realistic case where a warning for something like this is a good thing.
It doesn't matter if it's realistic or not, they are equivalent and should therefor give the same warning. The compiler can't know what reasons I have to write an expression in one or another way.

Quote:
The net result of thinking like you do is that hardly anybody is going to enable that warning.
I'm not thinking. The thinking was done by the people who designed the language and those who wrote the standards. I'm just using the language.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
How come we don't get a warning there ?
Because 1 is a constant known at compile time. x & 0xfffb is >>NOT<< a constant known at compile time.

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

dl8dtl wrote:
> The lack of common sense is sticking out like a sore thumb.

Shut up with that, please.

Compilers are stupid machines, which cannot employ something like
"common sense". If someone requests a 16-bit integer constant
by writing "~(1 << 7)", how on earth should the compiler notice
that the developer is actually not interested in its upper 8 bits?

If you don't like the way the C standard is written, then don't
use this language. Or, don't enable that warning.

The C standard does not require that warnings be based on brute force and ignorance.

The general form " &= "
should not produce the complained of warning in any standard C environment.
The implied conversion will never result in a change of value.
'Tain't necessary for the compiler to divine intent.
'Twould make sense for " &= 0x0F12" to produce a different warning.

"Demons after money.
Whatever happened to the still beating heart of a virgin?
No one has any standards anymore." -- Giles

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I still don't see te point. The example computes

    uint8_t & int
so that *obviously* bits are thrown away. If there would be no warning for that case, wouln't -Wconversion be pointless?

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
The general form " &= "
should not produce the complained of warning in any standard C environment.

I could possibly see that &= in this case might not produce a warning (though I don't agree), but Chauncey was also complaining that:

char x = ~(1 << 7);

produces a warning.

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

SprinterSB wrote:
I still don't see te point. The example computes
    uint8_t & int
so that *obviously* bits are thrown away. If there would be no warning for that case, wouln't -Wconversion be pointless?
The thrown away bits are all zeros. The mathematical value will not change.
gcc manual wrote:
-Wconversion'
Warn for implicit conversions that may alter a value.
In the case at hand, the implicit conversion will not alter the value.
Assigning -1 to an unsigned anything would alter its value and the warning would be appropriate.

"Demons after money.
Whatever happened to the still beating heart of a virgin?
No one has any standards anymore." -- Giles

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Koshchi wrote:
Quote:
How come we don't get a warning there ?
Because 1 is a constant known at compile time. x & 0xfffb is >>NOT<< a constant known at compile time.

It is known at compile time that 8 bits will be thrown away and that they will all be zeros.

If the "goal" of the compiler is to catch the high order bits of the constant being thrown away, it should do so even when assigning the result to an int. It doesn't:

unsigned char x;
unsigned int  i = x & 0xFFFB;
unsigned char c = x & 0xFFFB;

After those two statements, i and c hold the same value. The most significant byte in i is zero and the compiler should know that at compile time. Yet only the assignment to c is reported as a problem.

In other words, the compiler is happy to throw away 8 non-zero bits without warning. However, it does complain about 8 bits known to be zero being thrown away.

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
If the "goal" of the compiler is to catch the high order bits of the constant being thrown away, it should do so even when assigning the result to an int.
Why do you think that the high order bits of the constant is being thrown away when it is assigned to an int? Surely it is an int on both sides of the '='.

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ChaunceyGardiner wrote:

unsigned char x;
unsigned int  i = x & 0xFFFB;


Koshchi wrote:
Quote:
If the "goal" of the compiler is to catch the high order bits of the constant being thrown away, it should do so even when assigning the result to an int.
Why do you think that the high order bits of the constant is being thrown away when it is assigned to an int? Surely it is an int on both sides of the '='.

If you don't see that the most significant byte of the integer constant is thrown away here, you need to brush up on your C skills.

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

There is a big difference between "the bits in the upper byte of result after the operation are 0" and "the bits in the upper byte of the result are thrown away".

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Koshchi wrote:
I could possibly see that &= in this case might not produce a warning (though I don't agree), but Chauncey was also complaining that:

char x = ~(1 << 7);

produces a warning.

He's wrong on that one.
Assigning a negative number to a char might produce a positive number.

It might be worthwhile to define a less touchy version
of the same warning that allows discarding all ones.

"Demons after money.
Whatever happened to the still beating heart of a virgin?
No one has any standards anymore." -- Giles