None
EN
Clang Warnings
NULL
The LLVM Project Blog
Clang has two types of diagnostics, errors and warnings. Errors arise when the code does not conform to the language. Such things as missing semi-colons and mismatched braces prevent compilation and will cause Clang to emit an error message.On the other hand, warnings are emitted on questionable constructs on language conforming code. Over time, certain patterns have been determined to have a strong likelihood of being a programming mistake. Some examples of these include: order of operations confusion, mistaking similarly named language features, and easily made typos that still result in valid code.Although warnings may have false positives, the utility of finding bugs early usually outweigh their downsides. Keep reading for a demonstration of Clang's warnings, as well as a comparison to GCC's warnings.The following code consists of < 200 lines of code, one library, and one header file, only used for printing. It is legitimate C++ code and can be compiled into a program. Take a few moments and see if you can spot any bugs in the following code.main.cc#include "sort.h"#include <iostream>int main(int argc, char** argv) { int V[] = { 3, 4, 7, 10, 11, 1, 2, 0}; cout << "Unsorted numbers:" << endl; for( auto num : V ) cout << " " << num << endl; if (!sort(V, sizeof(V)/sizeof(V[0]))) { cout << "Sort failed." << endl; return 1; } cout << "Sorted numbers:" << endl; for( auto num : V ) cout << " " << num << endl; return 0;}sort.h#ifndef _EXPERIMENTAL_WARNINGS_SORT_H_#define _EXPERIMNETAL_WARNINGS_SORT_H_#include <iostream>#ifdef _NDEBUG#define ASSERT(cond) \ if(!cond) cout << __FILE__ << ":" << __LINE__ << " " << #cond << endl;#else#define ASSERT(cond) if (!cond) {}#endifenum SortType { unknown = 0, min_invalid = 3, bubble = 1, quick, insert};class Sort { public: Sort(int vec[], int size, bool sorted = false); bool IsSorted(); void Begin(SortType Type = unknown); private: void BubbleSort(); void QuickSort() { }; // Not implemented yet. void InsertSort() { }; // Not implemented yet. int* vec_; bool sorted_; int &size_;};static bool sort(int vec[], int size) { Sort sort(vec, size); sort.Begin(bubble); return sort.IsSorted();}#endif // _EXPERIMENTAL_WARNINGS_SORT_H_sort.cc#include <iostream>#include "sort.h"Sort::Sort(int vec[], int size, bool sorted) : sorted_(sorted_), vec_(vec), size_(size) { if (size > 50) ASSERT("!Vector too large. Number of elements:" + size); int sum; for (unsigned i = 0; !i == size; ++i) { int sum = sum + vec_[i]; ++i; } ASSERT(sum < 100 && "Vector sum is too high");}bool Sort::IsSorted() { return sort;}static bool CheckSort(int V[]) { bool ret; for (int i = 1; i != sizeof(V)/sizeof(V[0]); ++i) if (V[i] > V[i - 1]) ret = false; return ret;}static const char* TypeToString(SortType Type) { const char* ret; switch (Type) { case bubble: ret = "bubble"; case quick: ret = "quick"; case insert: ret = "insert"; } return ret;}void Sort::Begin(SortType Type) { cout << "Sort type: "; cout << Type == 0 ? "Unknown type, resorting to bubble sort" : TypeToString(Type); cout << endl; switch (Type) { default: bubble: BubbleSort(); break; quick: QuickSort(); break; insert: InsertSort(); break; } sorted_ = CheckSort(vec_);}void Sort::BubbleSort() { for (int i = 0; i < size_; ++i) { for (int j = 1; j < size_; ++i) { int a = vec_[j-1]; int b = vec_[j]; if (a > b); { vec_[j-1] = b; vec_[j] = a; } } }}Did you find any bugs? Many common problems are hard to spot from just reading the code. To make a better coding experience, Clang has many diagnostics that will flag these mistakes. The bugs in the code are detailed below.main.cc does not have any problems. It merely is a wrapper around the library so that a binary can be produced and run, although running it will not sort the array properly.sort.h is the header file to the library.1:#ifndef _EXPERIMENTAL_WARNINGS_SORT_H_2:#define _EXPERIMNETAL_WARNINGS_SORT_H_The first warning triggers on the first two lines. Header guards are used by libraries to prevent multiple #include’s of a file from producing redefinition errors. To work, the #ifndef and #define must use the same macro name.. The transposition of E and N produces different names and is an easy bug to overlook. Worse, this sort of bug can hide within headers and never produce a problem when singly included and then much later start producing problems when someone double includes this header. Clang has -Wheader-guard to catch this. GCC does not catch this.Next, examine the custom ASSERT macro used:7.#define ASSERT(cond) \8. if(!cond) cout << ...The problem is treating the macro parameter as a function parameter. Macro arguments are not evaluated. Instead, they are substituted in as typed. Thus, code as ASSERT(x == 5) becomes if(!x == 5) cout << ... The proper fix is to enclose the macro parameter in parentheses, as if (!(cond)) cout << ... This is caught by -Wlogical-not-parentheses. Being inside a macro definition, the warning will trigger when the macros are used with a note pointing back here. GCC has no equivalent for -Wlogical-not-parentheses.13:enum SortType {14: unknown = 0,15: min_invalid = 3,16:17: bubble = 1,18: quick,19: insert20:};In this enum, a few non-valid values are defined, then the valid enums listed. Valid enums use the auto increment to get their values. However, min_invalid and insert both have value 3. Luckily, -Wduplicate-enum will identify enums in this situation and point them out. GCC will not warn on this.On to sort.ccClass constructor:4:Sort::Sort(int vec[], int size, bool sorted)5: : sorted_(sorted_), vec_(vec), size_(size) {Members from sort.h:34: int* vec_;35: bool sorted_;36: int &size_;Checking the only constructor of the class, numerous problems can be seen here. First notice that the variables are declared vec_, sorted_, then size_, but in the constructor, they are listed as sorted_, vec_, then size_. The order of initialization is order they were declared, meaning vec_ is initialized before sorted_. There is no order dependence here, but -Wreorder will warn that the orders don’t match. GCC also has -Wreorder.Next, sorted_ is initialized with itself instead of with sorted. This leads to uninitialized value in sorted_, which is caught by the aptly named -Wuninitialized. For this case, GCC has -Wself-assign and -Wself-init.Finally, notice that size_ is declared as a reference but size is not passed by reference. size only lives until the end of the constructor while the reference size_ will continue to point to it. -Wdangling-field catches this problem.7: ASSERT("!Vector too large. Number of elements:" + size);Two problems here. Adding an integer to a string literal does not concatenate the two together. Instead, since a string literal is a pointer of type const char *, this actually performs pointer math. With a sufficiently integer, this can even cause the pointer to go past the end of the string into some other memory. -Wstring-plus-int warns on this case. GCC has no equivalent warning.7: ASSERT("!Vector too large. More than 50 elements.");When fixed, another problem arises. A common pattern is to include a string literal describing the assert. If the assert is always to fire, then expression should evaluate to false. The string literal evaluates to true, so just negate it to get a false value, right? Well, a number of common typos can happen.These values evaluate true:"true""false""!true""!false""any string"These values evaluate to false:!"true"!"false"!"any string"!"!any string"Due to that, -Wstring-conversion will warn when a string literal is converted to a true boolean value. Use ASSERT(false && “string”) or ASSERT(0 && “string”)