views:

441

answers:

6

Our static analysis tool complains about a "useless type qualifier on return type" when we have prototypes in header files such as:

const int foo();

We defined it this way because the function is returning a constant that will never change, thinking that the API seemed clearer with const in place.

I feel like this is similar to explicitly initializing global variables to zero for clarity, even though the C standard already states that all globals will be initialized to zero if not explicitly initialized. At the end of the day, it really doesn't matter. (But the static analysis tool doesn't complain about that.)

My question is, is there any reason that this could cause a problem? Should we ignore the errors generated by the tool, or should we placate the tool at the possible cost of a less clear and consistent API? (It returns other const char* constants that the tool doesn't have a problem with.)

+4  A: 

You can use a different technique to illustrate your intent without making the tools unhappy.

#define CONST_RETURN

CONST_RETURN int foo();

You don't have a problem with const char * because that's declaring a pointer to constant chars, not a constant pointer.

Mark Ransom
But given that there's no compiler checking of that CONST_RETURN claim, isn't it better to have a comment? That way, at least it doesn't *look* like something the compiler must have checked.
Steve314
Yes, a comment might be even better.
Mark Ransom
There are a few APIs/libraries I've seen that have done something similar to this. If you have a function that always returns the same number, `#define foo() VALUE` might be the trick.
Carl Norum
Macros are bad.
David Thornley
Macros are primitive? Yes. Macros are ugly? Often. Macros are dangerous? Sometimes. Macros are bad? Macros are a tool, and tools can be used badly, but I wouldn't give them a blanket condemnation.
Mark Ransom
+13  A: 

It's usually better for your code to describe as accurately as possible what's going on. You're getting this warning because the const in const int foo(); is basically meaningless. The API only seems clearer if you don't know what the const keyword means. Don't overload meaning like that; static is bad enough as it is, and there's no reason to add the potential for more confusion.

const char * means something different than const int does, which is why your tool doesn't complain about it. The former is a pointer to a constant string, meaning any code calling the function returning that type shouldn't try to modify the contents of the string (it might be in ROM for example). In the latter case, the system has no way to enforce that you not make changes to the returned int, so the qualifier is meaningless. A closer parallel to the return types would be:

const int foo();
char * const foo2();

which will both cause your static analysis to give the warning - adding a const qualifier to a return value is a meaningless operation. It only makes sense when you have a a reference parameter (or return type), like your const char * example.

In fact, I just made a little test program, and GCC even explicitly warns about this problem:

test.c:6: warning: type qualifiers ignored on function return type

So it's not just your static analysis program that's complaining.

Carl Norum
Agreed. My instant reaction to a const return type is to assume the return is a reference/pointer to a shared buffer that shouldn't be changed. My mental model is that const applies to the container (e.g. variable), not the content. In "const char*" for example, the const applies to the pointed-to string, whereas if you have "const int i = 5;", you can still write "i + 1" in an expression - you can work with the value so long as you don't try to change the variable. With a simple int return, you don't really have a container - only a value.
Steve314
I guess people get confused because of the weird behaviour of the `const` keyword. I mean that `const char *c` and `char const *c` are the same thing. If the former syntax weren't legal, things would be a lot less confusing. We'd only have `char const *c` and `char * const c` and everyone would know what's going on. Maybe I'm thinking about it too much.
Carl Norum
Okay, I'm convinced. Thanks for pointing out the GCC warning. I wish it would have been an error rather than a warning, so gray areas like this don't come up.
Mike
Among other things, `const` doesn't in general mean "this is never going to change". `const` usually means "you (API client) cannot change this". The distinction is obvious when you deal with pointers-to-const.
Pavel Minaev
+1 @Pavel. That's a good, concise, explanation.
Carl Norum
@Mike, it's not an error because it doesn't cause any difference in program execution. Most compilers will let you turn warnings into errors if you so choose, however.
Carl Norum
@Carl: "const char * ... is a pointer to a constant string". Is it not a pointer to a constant char, or alternatively, the address of the (constant) first element of a char array/string?
Steve Melnikoff
Yes, arguably the const applies to only the first character of the string. In practice it's almost always the case that `char *` points to a string (C strings simply being null-terminated arrays of characters).
Carl Norum
+3  A: 

const int foo() is very different from const char* foo(). const char* foo() returns an array (usually a string) whose content is not allowed to change. Think about the difference between:

 const char* a = "Hello World";

and

const int b = 1;

a is still a variable and can be assigned to other strings that can't change whereas b is not a variable. So

const char* foo();
const char* a = "Hello World\n";
a = foo();

is allowed but

const int bar();
const int b = 0;
b = bar();

is not allowed, even with the const declaration of bar().

atlpeg
`const int b = bar()` is allowed, because you can always initialize a `const` variable. Do you mean `const int b; b = bar()`?
Pavel Minaev
You're right. Answer corrected.
atlpeg
+3  A: 

Ignoring the const for now, foo() returns a value. You can do

int x = foo();

and assign the value returned by foo() to the variable x, in much the same way you can do

int x = 42;

to assign the value 42 to variable x.
But you cannot change the 42 ... or the value returned by foo(). Saying that the value returned from foo() cannot be changed, by applying the const keyword to the type of foo() accomplishes nothing.

Values cannot be const (or restrict, or volatile). Only objects can have type qualifiers.


Contrast with

const char *foo();

In this case, foo() returns a pointer to an object. The object pointed to by the value returned can be qualified const.

pmg
+2  A: 

The int is returned by copy. It may be a copy of a const, but when it is assigned to something else, that something by virtue of the fact that it was assignable, cannot by definition be a const.

The keyword const has specific semantics within the language, whereas here you are misusing it as essentially a comment. Rather than adding clarity, it rather suggests a misunderstanding of the language semantics.

Clifford
What does const int PI=3.14159; int myvalue = 2 * PI; do? Are you saying that PI should not be declared const? If the value returned by the function is not going to change, it is a constant, therefore it is quite reasonable to declare it as such.
Jason Williams
@Jason: quite reasonable but also quite redundant, which is why the tools are treating this as a warning and not an error. In your example the `const` is *not* redundant.
Mark Ransom
That's not the same thing. If I have a function `getPi()` that just has `return PI` in it, the returned value is not `const`. It *cannot* be `const` because of the pass-by-value (and return-by-value) language conventions.
Carl Norum
There is no debate as to whether it is semantically correct - even the original question makes it clear that it isn't. My point is that as long as "unnecessary" code does not alter the logic/flow of the program, and does not confuse the reader by assigning an unconventional and unintuitive meaning to an existing token, I don't have a problem with a programming team using such a convention. In this case (unlike others I could cite), it appears to be a rather harmless team convention, which would not confuse or mislead an "external" reader should they come across it.
Jason Williams
@Jason: We will have to agree to disagree, because I believe that it is potentially confusing. But more seriously suggests a lack of expertise and understanding by the development team. For example if, as a third party I received such code, it would cause me concern over the general quality, since it may indicate lack of experience and knowledge of the language semantics. From your first comment it seems that perhaps you are in a similar situation; because as has been pointed out, initialisation of a const is not the same as return-by-value qualified as const.
Clifford
+2  A: 

Yes. I would advise writing code "explicitly", because it makes it clear to anyone (including yourself) when reading the code what you meant. You are writing code for other programmers to read, not to please the whims of the compiler and static analysis tools!

(However, you do have to be careful that any such "unnecessary code" does not cause different code to be generated!)

Some examples of explicit coding improving readability/maintainability:

  • I place brackets around portions of arithmetic expressions to explicitly specify what I want to happen. This makes it clear to any reader what I meant, and saves me having to worry about (or make ay mistakes with) precedence rules:

    int a = b + c * d / e + f;      // Hard to read- need to know precedence
    int a = b + ((c * d) / e) + f;  // Easy to read- clear explicit calculations
    

  • In C++, if you override a virtual function, then in the derived class you can declare it without mentioning "virtual" at all. Anyone reading the code can't tell that it's a virtual function, which can be disastrously misleading! However you can safely use the virtual keyword:

    virtual int MyFunc()
    and this makes it clear to anyone reading your class header that this method is virtual. (This "C++ syntax bug" is fixed in C# by requiring the use of the "override" keyword in this case - more proof if anyone needed it that missing out the "unnecessary virtual" is a really bad idea)

These are both clear examples where adding "unnecessary" code will make the code more readable and less prone to bugs.

Jason Williams
You're not talking about the same thing. As @pmg says, **values** cannot be `const` or anything else. Only objects can. Saying that it's valuable to stick a `const` label on something that is by definition not `const` is clearly a bad idea. It confuses the semantics unnecessarily.
Carl Norum