www.digitalmars.com

D Programming Language 1.0


Last update Sun Dec 30 20:34:43 2012

Warnings

Depending on one's point of view, warnings are either a symptom of broken language design or a useful 'lint' like tool to analyze code and look for potential trouble spots. Most of the time, those trouble spots will be legitimate code intended to be that way. More rarely, it may indicate an unintentional error on the part of the programmer.

Warnings are not a defined part of the D Programming Language. They exist at the discretion of the compiler vendor, and will most likely vary from vendor to vendor. All constructs for which an implementation may generate a warning message are legal D code.

These are the warnings generated by the Digital Mars D compiler when the -w switch is thrown. Most have generated some spirited debate as to whether it should be a warning, an error, and what the correct way to write D code in these situations should be. Since no consensus emerged, they appear as optional warnings, with alternatives on how to write correct code.

warning - implicit conversion of expression expr of type type to type can cause loss of data

D follows the C and C++ rules regarding default integral promotion in expressions. This is done for both compatibility reasons and because modern CPUs are designed to execute such semantics efficiently. These rules also include implicit narrowing rules, where the result of such can be cast back into a smaller type, perhaps losing significant bits in the process:

byte a,b,c;
...
a = b + c;

The b and c are both promoted to type int using the default integral promotion rules. The result of the add is then also int. To be assigned to a, that int gets implicitly converted to a byte.

Whether it is a bug or not in the program depends on if the values for b and c can possibly cause an overflow, and if that overflow matters or not to the algorithm. The warning can be resolved by:

  1. Inserting a cast:
    a = cast(byte)(b + c);
    
    This eliminates the warning, but since casting is a blunt instrument that bypasses type checking, this can mask another bug that could be introduced if the types of a, b or c change in the future, or if their types are set by a template instantiation. (In generic code, the cast(byte) would probably be cast(typeof(a))).
  2. Changing the type of a to int. This is generally a better solution, but of course may not work if it must be a smaller type, which leads us to:
  3. Doing a runtime check for overflow:
    byte a,b,c;
    int tmp;
    ...
    tmp = b + c;
    assert(cast(byte)tmp == tmp);
    a = cast(byte)tmp;
    
    This ensures that no significant bits get lost.

Some proposed language solutions are:

  1. Having the compiler automatically insert the equivalent of the option 3 runtime check.
  2. Add a new construct, implicit_cast(type)expression that is a restricted form of the general cast, and will only work where implicit casts would normally be allowed.
  3. Implement the implicit_cast as a template.

warning - array 'length' hides other 'length' name in outer scope

Inside the [ ] of an array indexing expression or slice, the variable length gets defined and set to be the length of that array. The scope of the length is from the opening '[' to the closing ']'.

If length was declared as a variable name in an outer scope, that version may have been intended for use between the [ ]. For example:

char[10] a;
int length = 4;
...
return a[length - 1];	// returns a[9], not a[3]

The warning can be resolved by:

  1. Renaming the outer length to another name.
  2. Replacing the use of length within the [ ] with a.length.
  3. If length is at global or class scope, and that was the one intended to be used, use .length or this.length to disambiguate.

Some proposed language solutions are:

  1. Make the length a special symbol or a keyword instead of an implicitly declared variable.

warning - no return at end of function

Consider the following:

int foo(int k, Collection c)
{
    foreach (int x; c)
    {
	if (x == k)
	    return x + 1;
    }
}

and assume that the nature of the algorithm is that x will always be found in c, so the foreach never falls through the bottom of the code. There is no return at the close of the function, because that point is not reachable and any return statement would be dead code. This is perfectly legitimate code. The D compiler, to help with ensuring the robustness of such code, will ensure it is correct by automatically inserting an assert(0); statement at the close of the function. Then, if there's an error in the assumption that the foreach will never fall through, it will be caught at runtime with an obvious assertion failure, which is better than the code just falling off the end causing erratic, arbitrary failures.

D's behavior on this, however, is not common to other languages, and some programmers will be acutely uncomfortable with relying on this, or they wish to see the error at compile time rather than runtime. They wish to see something explicit expressed in the source code that the foreach cannot fall through. Hence the warning.

The warning can be resolved by:

  1. Putting a return statement at the close of the function, returning some arbitrary value (after all, it will never be executed):
    int foo(int k, Collection c)
    {
        foreach (int x; c)
        {
    	if (x == k)
    	    return x + 1;
        }
        return 0;	// suppress warning about no return statement
    }
    
    While doing this is surprisingly common, and happens when the programmer is inexperienced or in a hurry, it is a spectacularly bad solution. The trouble happens when the foreach does fall through due to a bug, then instead of the bug being detected, now the function returns an unexpected value, which may cause other problems or go undetected. There's another issue with the maintenance programmer who sees this, and upon analyzing the code's behavior, wonders why there's a return statement that will never be executed. (This can be resolved with comments, but comments are always missing, out of date, or just plain wrong.) Dead code is always a confusing problem to maintenance programming, so deliberately inserting it is a bad idea.
  2. A better solution is to make explicit what the D language does implicitly - put an assert there:
    int foo(int k, Collection c)
    {
        foreach (int x; c)
        {
    	if (x == k)
    	    return x + 1;
        }
        assert(0);
    }
    
    Now, if the foreach does fall through, the error will be detected. Furthermore, it is self-documenting.
  3. Another alternative is to do a throw with a custom error class and a more user friendly message:
    int foo(int k, Collection c)
    {
        foreach (int x; c)
        {
    	if (x == k)
    	    return x + 1;
        }
        throw new MyErrorClass("fell off the end of foo()");
    }
    

warning - switch statement has no default

Similar to the no return statement warning is the no default in a switch warning. Consider:

switch (i)
{
    case 1: dothis(); break;
    case 2: dothat(); break;
}

According to the D language semantics, this means the only possible values for i are 1 and 2. Any other values represent a bug in the program. To detect this bug, the compiler implements the switch as if it were written:

switch (i)
{
    case 1: dothis(); break;
    case 2: dothat(); break;
    default: throw new SwitchError();
}

This is quite different from C and C++ behavior, which is as if the switch were written:

switch (i)
{
    case 1: dothis(); break;
    case 2: dothat(); break;
    default: break;
}

The potential for bugs with that is high, as it is a common error to accidentally omit a case, or to add a new value in one part of the program and overlook adding a case for it in another part. Although D will catch this error at runtime by throwing an instance of std.switcherr.SwitchError, some prefer at least a warning so such errors can be caught at compile time.

The warning can be resolved by:

  1. Inserting an explict default of the form:
    switch (i)
    {
        case 1: dothis(); break;
        case 2: dothat(); break;
        default: assert(0);
    }
    
  2. As in the missing return, a default with a throw of a custom error class can be used.

warning - statement is not reachable

Consider the following code:

int foo(int i)
{
    return i;
    return i + 1;
}

The second return statement is not reachable, i.e. it is dead code. While dead code is poor style in released code, it can legitimately happen a lot when rapidly trying to isolate down a bug or experiment with different bits of code.

The warning can be resolved by:

  1. Commenting out the dead code with /+ ... +/ comments:
    int foo(int i)
    {
        return i;
        /+
    	return i + 1;
         +/
    }
    
  2. Putting the dead code inside a version(none) conditional:
    int foo(int i)
    {
        return i;
        version (none)
        {
    	return i + 1;
        }
    }
    
  3. Only compile with warnings enabled when doing release builds.




Forums | Comments |  D  | Search | Downloads | Home