widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

Review: Approve

We could add a codecheck that runs clang-format and complains if the output is different to what clang format produces. But that might be brittle as clang-format is slightly changing between versions IMHO.


One nit, otherwise lgtm.

Diff comments:

> 
> === removed file 'cmake/codecheck/rules/missing_padding'
> --- cmake/codecheck/rules/missing_padding	2014-07-16 08:23:42 +0000
> +++ cmake/codecheck/rules/missing_padding	1970-01-01 00:00:00 +0000
> @@ -1,378 +0,0 @@
> -#!/usr/bin/env python -tt

keep this one and just allow Type*/Type& ?

> -# encoding: utf-8
> -#
> -
> -"""
> -Checking for forbidden padding.
> -TypeName* and TypeName& are okay, as are TypeName * and TypeName &.
> -"""
> -
> -error_msg="Missing padding for operator. Needs a space before and after."
> -
> -import re
> -
> -# It is easier to search for invalids than for 'NOT' invalids
> -regexp = re.compile(r"""(?x)
> -([^\s](?:(?:&&)|(?:\|\|)))|
> -((?:(?:&&)|(?:\|\|))[^\s])|
> -([^\s>(&*:!{+\-[.](?<!operator)[*&][^*\s>)},])|          # Type*, Type&, operator
> -([+\-*/|^&!<]?={1,2}[^\s=])|   # '+= a', 'a== a', 'a!= a'
> -([^\s=!+-/^*/&|^<>r%][+\-*/^&|!<>%]?={1,2})|   # ' +=a', ' ==a', ' !=a', doesn't match 'r=' for operator=
> -
> -([^-<]>[^\s=&>*(:,);])|    # 'a >b'
> -
> -([^\s][/^\%](?!=))|    # '/ a'
> -([/^\%][^=\s])|
> -([^\s\-eE([{]-[^\s=\->])|    # -> and -= and unary -, also 8e-1
> -([^\s+]\+[^\s=+])      # +=
> -"""
> -)
> -
> -def match_function( lines, fn ):
> -    errors = []
> -    for lineno,line in enumerate(lines):
> -        line = line.strip()
> -        if not len(line) or line[0] == '#':
> -            continue
> -
> -        lineno += 1
> -
> -        if regexp.search(line):
> -            errors.append( (fn, lineno, error_msg) )
> -
> -    return errors
> -
> -strip_comments_and_strings = True
> -
> -evaluate_matches = match_function
> -
> -forbidden = [
> -    " a== b",
> -    " a ==b",
> -    " a !=b",
> -    " a!= b",
> -    "a>= b",
> -    "a >=b",
> -    "b<= a",
> -    "b <=a",
> -    #"b< a",  could also be std::vector<a
> -    #"b <a", could also be std::vector<hai>
> -    #"b> a", could also be std::vector<hallo> hi;
> -    "b >a",
> -
> -    ' a&& b',
> -    ' a &&b',
> -    ' a|| b',
> -    ' a ||b',
> -
> -    'char a = a^ a;',
> -    'char a = a ^a;',
> -
> -    # Eriks tests
> -    ## missing_padding.cc
> -    'char a=a;',
> -    'char a = a^a;',
> -    'char a = a%a;',
> -    'char a = a/a;',
> -    'char a = a*a;',
> -    'char a = a+a;',
> -    'char a = a-a;',
> -    'char A =A;',
> -    'char A = A^A;',
> -    'char A = A%A;',
> -    'char A = A/A;',
> -    'char A = A*A;',
> -    'char A = A+A;',
> -    'char A = A-A;',
> -
> -    'char b=b;',
> -    'char b = b^b;',
> -    'char b = b%b;',
> -    'char b = b/b;',
> -    'char b = b*b;',
> -    'char b = b+b;',
> -    'char b = b-b;',
> -    'char B =B;',
> -    'char B = B^B;',
> -    'char B = B%B;',
> -    'char B = B/B;',
> -    'char B = B*B;',
> -    'char B = B+B;',
> -    'char B = B-B;',
> -
> -    'char _ =_;',
> -    'char _ = _^_;',
> -    'char _ = _%_;',
> -    'char _ = _/_;',
> -    'char _ = _*_;',
> -    'char _ = _+_;',
> -    'char _ = _-_;',
> -
> -    'char x0 =010;',
> -    'char x0 = 010^010;',
> -    'char x0 = 010%010;',
> -    'char x0 = 010/010;',
> -    'char x0 = 010*010;',
> -    'char x0 = 010+010;',
> -    'char x0 = 010-010;',
> -
> -    'char x1 =1;',
> -    'char x1 = 1^1;',
> -    'char x1 = 1%1;',
> -    'char x1 = 1/1;',
> -    'char x1 = 1*1;',
> -    'char x1 = 1+1;',
> -    'char x1 = 1-1;',
> -
> -    'char const * str ="";',
> -
> -    'char ch =' ';',
> -
> -    'a^= a;',
> -    'a ^=a;',
> -    'a^=a;',
> -    'a/g= a;',
> -    'a /g=a;',
> -    'a/g=a;',
> -    'a*= a;',
> -    'a *=a;',
> -    'a*=a;',
> -    'a-= a;',
> -    'a -=a;',
> -    'a-=a;',
> -    'a+= a;',
> -    'a +=a;',
> -    'a+=a;',
> -    'a%=a;',
> -    'a%= a;',
> -    'a %=a;',
> -
> -    'A^= A;',
> -    'A ^=A;',
> -    'A^=A;',
> -    'A/g= A;',
> -    'A /g=A;',
> -    'A/g=A;',
> -    'A*= A;',
> -    'A *=A;',
> -    'A*=A;',
> -    'A-= A;',
> -    'A -=A;',
> -    'A-=A;',
> -    'A+= A;',
> -    'A +=A;',
> -    'A+=A;',
> -
> -    '_^= _;',
> -    '_ ^=_;',
> -    '_^=_;',
> -    '_/g= _;',
> -    '_ /g=_;',
> -    '_/g=_;',
> -    '_*= _;',
> -    '_ *=_;',
> -    '_*=_;',
> -    '_-= _;',
> -    '_ -=_;',
> -    '_-=_;',
> -    '_+= _;',
> -    '_ +=_;',
> -    '_+=_;',
> -
> -]
> -
> -allowed = [
> -    "char** argv",
> -    "AnimationGfx(const IImageLoader&, AnimationData const * data);",
> -    "RoutingNode* a;",
> -    "Blahtype& b;",
> -    "std::vector<RoutingNode*> & nodes",
> -
> -    'type_a& aa = aa;',
> -    'type_A& AA = AA;',
> -    'type_b& bb = bb;',
> -    'type_B& BB = BB;',
> -    'type_c& cc = cc;',
> -    'type_C& CC = AA;',
> -    'type_d& dd = dd;',
> -    'type_D& DD = DD;',
> -    'type_e& ee = ee;',
> -    'type_E& EE = EE;',
> -    'type_f& ff = ff;',
> -    'type_F& FF = FF;',
> -    'type_g& gg = gg;',
> -    'type_G& GG = GG;',
> -    'type_h& hh = hh;',
> -    'type_H& HH = HH;',
> -    'type_i& ii = ii;',
> -    'type_I& II = II;',
> -    'type_j& jj = jj;',
> -    'type_J& JJ = JJ;',
> -    'type_k& kk = kk;',
> -    'type_K& KK = KK;',
> -    'type_l& ll = ll;',
> -    'type_L& LL = LL;',
> -    'type_m& mm = mm;',
> -    'type_M& MM = MM;',
> -    'type_n& nn = nn;',
> -    'type_N& NN = NN;',
> -    'type_o& oo = oo;',
> -    'type_O& OO = OO;',
> -    'type_p& pp = pp;',
> -    'type_P& PP = PP;',
> -    'type_q& qq = qq;',
> -    'type_Q& QQ = QQ;',
> -    'type_r& rr = rr;',
> -    'type_R& RR = RR;',
> -    'type_s& ss = ss;',
> -    'type_S& SS = SS;',
> -    'type_t& tt = tt;',
> -    'type_T& TT = TT;',
> -    'type_u& uu = uu;',
> -    'type_U& UU = UU;',
> -    'type_v& vv = vv;',
> -    'type_V& VV = VV;',
> -    'type_w& ww = ww;',
> -    'type_W& WW = WW;',
> -    'type_x& xx = xx;',
> -    'type_X& XX = XX;',
> -    'type_y& yy = yy;',
> -    'type_Y& YY = YY;',
> -    'type_z& zz = zz;',
> -    'type_Z& ZZ = ZZ;',
> -    'type_a* aaa;',
> -    'type_b* bbb;',
> -    'type_c* ccc;',
> -    'type_d* ddd;',
> -    'type_e* eee;',
> -    'type_f* fff;',
> -    'type_g* ggg;',
> -    'type_h* hhh;',
> -    'type_i* iii;',
> -    'type_j* jjj;',
> -    'type_k* kkk;',
> -    'type_l* lll;',
> -    'type_m* mmm;',
> -    'type_n* nnn;',
> -    'type_o* ooo;',
> -    'type_p* ppp;',
> -    'type_q* qqq;',
> -    'type_r* rrr;',
> -    'type_s* sss;',
> -    'type_t* ttt;',
> -    'type_u* uuu;',
> -    'type_v* vvv;',
> -    'type_w* www;',
> -    'type_x* xxx;',
> -    'type_y* yyy;',
> -    'type_z* zzz;',
> -    'type_A* AAA;',
> -    'type_B* BBB;',
> -    'type_C* CCC;',
> -    'type_D* DDD;',
> -    'type_E* EEE;',
> -    'type_F* FFF;',
> -    'type_G* GGG;',
> -    'type_H* HHH;',
> -    'type_I* III;',
> -    'type_J* JJJ;',
> -    'type_K* KKK;',
> -    'type_L* LLL;',
> -    'type_M* MMM;',
> -    'type_N* NNN;',
> -    'type_O* OOO;',
> -    'type_P* PPP;',
> -    'type_Q* QQQ;',
> -    'type_R* RRR;',
> -    'type_S* SSS;',
> -    'type_T* TTT;',
> -    'type_U* UUU;',
> -    'type_V* VVV;',
> -    'type_W* WWW;',
> -    'type_X* XXX;',
> -    'type_Y* YYY;',
> -    'type_Z* ZZZ;',
> -    "RoutingNode* a;",
> -    "Blahtype& b;",
> -    "RoutingNode * a;",
> -    "Blahtype & b;",
> -    "/** Some comment **/",
> -    "std::vector<RoutingNode *> & nodes",
> -    "if (a && b)",
> -    "\tOpen.push(&start);",
> -    "\t\t*reinterpret_cast<std::vector<RoutingNode *>*>(&m_flags);",
> -    "std::vector<RoutingNode *>& nodes",
> -        "/* warehouses can determine max idle priority*/",
> -        "blah // I *wanna* do that",
> -    "&*m_stack.rbegin()",
> -
> -    "(std::set<std::string>, j.current->second, i)",
> -
> -    # > <
> -    "r = (palette[i].r * shade) >> 8;",
> -    "std::vector<High>",
> -    "std::vector<State>::iterator it = m_stack.end();",
> -
> -    "v->get_string()", # pointers are fine
> -
> -    # Allow includes
> -    "#include <sys/types.h>",
> -
> -
> -    # Ignore strings & comments
> -    "\tif (*i.current == '%') {",
> -    "if (calc_linewidth(font, cur_word) > (max_width /*/ 2*/)) {",
> -
> -
> -    # * Pointer to pointer
> -    "func(int **r);",
> -    "void (a::*b)",
> -    "if (!*ppgrid) {",
> -    "void MiniMap::toggle(Layers button) {*m_view.m_flags ^= button;}",
> -    "m_loaded_obj[&object] = false;",
> -    "m_loaded_obj[*object] = false;",
> -    "(_callback_argument_this.*_callback_function)();",
> -    "friend struct Table<void *>;",
> -
> -    # Unary minus
> -    "-1",
> -    "func(-1);",
> -    "8.46126929e-5",
> -    "-8.46126929e-5",
> -
> -    # Operator=
> -    "int someclass::operator= (int)",
> -    "int someclass::operator* (int)",
> -    "int someclass::operator== (int)",
> -
> -    # increment, decrement
> -    "incr++;",
> -    "++incr;",
> -    "--decr;",
> -    "decr--;",
> -    "++*digit_to_increment;",
> -
> -    # +=, -=
> -    "result += it",
> -    "result -= it",
> -
> -    # Logical
> -    "a && b",
> -    "a || b",
> -
> -    # compares
> -    "a == b",
> -    "a != b",
> -    "a >= b",
> -    "b <= a",
> -    "b < a",
> -    "b > a",
> -    "AreaWatcher(const Player_Area<>);",
> -    "void func (Player_Area<Area>);",
> -
> -    'a %= a;',
> -
> -    'Texture const &  /* f_r_texture */',
> -    'int32_t estimate(Map & /* map */, FCoords /* pos */) const {return 0;}',
> -
> -]


-- 
https://code.launchpad.net/~widelands-dev/widelands/clang-codecheck/+merge/284586
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/clang-codecheck.