Vous vous souvenez peut-être de l' annonce récente d'un nouvel analyseur statique pour Go appelé Go -Critic .

J'ai vérifié le projet golang / go avec lui et envoyé des correctifs qui corrigent certains problèmes trouvés là-bas.

Dans cet article, nous analyserons le code corrigé et nous serons également motivés à envoyer encore plus de telles modifications à Go.

Pour les plus impatients: une liste actualisée des trophées .

dupSubExpr

Nous faisons tous des erreurs et, bien souvent, par inattention. Go, étant une langue dans laquelle vous devez parfois écrire du code ennuyeux et passe-partout, contribue parfois aux erreurs de frappe et / ou de copier / coller.

CL122776 contient un correctif pour un bogue trouvé par dupSubExpr :

 func (a partsByVarOffset) Less(i, j int) bool { - return varOffset(a.slots[a.slotIDs[i]]) < varOffset(a.slots[a.slotIDs[i]]) + return varOffset(a.slots[a.slotIDs[i]]) < varOffset(a.slots[a.slotIDs[j]]) // ^__________________________________^ } 

Faites attention à l'index à gauche et à droite. Avant la correction, les LHS et RHS de l'opérateur < étaient identiques, et dupSubExpr travaillé pour dupSubExpr .

commentedOutCode

Si votre projet est parrainé par un système de contrôle de version , au lieu de désactiver le code en l'enveloppant dans un commentaire, il vaut la peine de le supprimer complètement. Il existe des exceptions, mais le plus souvent, un tel code "mort" interfère, confond et peut masquer des erreurs.

commentedOutCode a pu trouver un tel fragment intéressant ( CL122896 ):

 switch arch.Family { // ...  case clause. case sys.I386: return elf.R_386(nr).String() case sys.MIPS, sys.MIPS64: // return elf.R_MIPS(nr).String() // <- 1 case sys.PPC64: // return elf.R_PPC64(nr).String() // <- 2 case sys.S390X: // return elf.R_390(nr).String() // <- 3 default: panic("unreachable") } 

Il y a un commentaire un peu plus haut:

 // We didn't have some relocation types at Go1.4. // Uncomment code when we include those in bootstrap code. 

Si vous basculez vers la branche go1.4 et supprimez ces 3 lignes du commentaire, le code ne sera pas compilé, mais si vous les décommentez sur l'assistant, tout fonctionnera.

En règle générale, le code caché dans un commentaire nécessite la suppression ou l'activation inverse.

Il est utile, de temps en temps, de visiter ces échos du passé dans votre code.

boolExprSimplify

Parfois, les gens écrivent du code bizarre. Cela me semble peut-être, mais les expressions logiques ( booléennes ) semblent parfois particulièrement étranges.

Go a un excellent backend assembleur x86 (ici une déchirure est tombée), mais ARM a vraiment mal fait:

 if !(o1 != 0) { break } 

"Si non o1 n'est pas égal à 0" ... La double négation est un classique. Si cela vous a plu, je vous invite à vous familiariser avec le CL123377 . Là, vous pouvez voir la version corrigée.

 - if !(o1 != 0) { + if o1 == 0 { 

boolExprSimplify vise des simplifications qui augmentent la lisibilité (et l'optimiseur Go aurait résolu le problème des performances sans lui).

underef

Si vous utilisez Go à partir de ses versions antérieures, vous pouvez vous souvenir des points-virgules obligatoires, de l'absence de déréférencement automatique des pointeurs et d'autres fonctionnalités qui sont aujourd'hui presque impossibles à voir dans le nouveau code.

Dans l'ancien code, vous pouvez toujours voir quelque chose comme ceci:

 // -    : buf := (*bufp).ptr() // ...     : buf := bufp.ptr() 

Plusieurs déclencheurs d'analyseurs sous- correctifs corrigés dans CL122895 .

appendCombine

Vous savez peut-être que l' append peut prendre plusieurs arguments comme éléments à ajouter à la tranche cible. Dans certaines situations, cela vous permet d'améliorer légèrement la lisibilité du code, mais, ce qui peut être plus intéressant, cela peut également accélérer votre programme, car le compilateur ne supprime pas les appels d' append compatibles ( cmd / compile: combiner les appels d'ajout ).

Dans Go, la vérification appendCombine a trouvé la section suivante:

 - for i := len(ip) - 1; i >= 0; i-- { - v := ip[i] - buf = append(buf, hexDigit[v&0xF]) - buf = append(buf, '.') - buf = append(buf, hexDigit[v>>4]) - buf = append(buf, '.') - } + for i := len(ip) - 1; i >= 0; i-- { + v := ip[i] + buf = append(buf, hexDigit[v&0xF], + '.', + hexDigit[v>>4], + '.') + } 

 name old time/op new time/op delta ReverseAddress-8 4.10µs ± 3% 3.94µs ± 1% -3.81% (p=0.000 n=10+9) 

Détails dans CL117615 .

rangeValCopy

Ce n'est un secret pour personne que les valeurs répétées dans une boucle de range sont copiées. Pour les petits objets, disons, moins de 64 octets, vous ne remarquerez peut-être même pas cela. Cependant, si un tel cycle se trouve sur un chemin «chaud» ou, que vous parcourez, contient un très grand nombre d'éléments, la surcharge peut être tangible.

Go a un éditeur de liens plutôt lent (cmd / link), et sans changements importants dans son architecture, un gain de performances élevé ne peut pas être atteint. Mais vous pouvez alors réduire légèrement son inefficacité à l'aide de microoptimisations. Chaque pourcentage ou deux compte.

La vérification rangeValCopy a trouvé plusieurs cycles avec la copie de données indésirables à la fois. Voici les plus intéressants d'entre eux:

 - for _, r := range exports.R { - d.mark(r.Sym, nil) - } + for i := range exports.R { + d.mark(exports.R[i].Sym, nil) + } 

Au lieu de copier R[i] à chaque itération, nous nous tournons uniquement vers le seul membre qui nous intéresse, Sym .

 name old time/op new time/op delta Linker-4 530ms ± 2% 521ms ± 3% -1.80% (p=0.000 n=17+20) 

La version complète du patch est disponible sur: CL113636 .

namedConst

Dans Go, malheureusement, les constantes nommées, même assemblées en groupes, ne sont pas interconnectées et ne forment pas une énumération ( proposition: spec: ajouter un support d'énumération typé ).

Un problème est de transposer des constantes non typées dans le type que vous souhaitez utiliser comme énumération.

Supposons que vous définissiez un type de Color , sa valeur est const ColDefault Color = 0 .
Lequel de ces deux extraits de code préférez-vous?

 // (A) if color == 0 { return colorBlack } // (B) if color == colorDefault { return colorBlack } 

Si (B) semble plus approprié, la vérification de namedConst vous aidera à suivre l'utilisation des valeurs constantes nommées, en contournant la constante nommée elle-même.

Voici comment la méthode context.mangle du package html/template été transformée:

  s := templateName + "$htmltemplate_" + c.state.String() - if c.delim != 0 { + if c.delim != delimNone { s += "_" + c.delim.String() } - if c.urlPart != 0 { + if c.urlPart != urlPartNone { s += "_" + c.urlPart.String() } - if c.jsCtx != 0 { + if c.jsCtx != jsCtxRegexp { s += "_" + c.jsCtx.String() } - if c.attr != 0 { + if c.attr != attrNone { s += "_" + c.attr.String() } - if c.element != 0 { + if c.element != elementNone { s += "_" + c.element.String() } return s 

Soit dit en passant, parfois sur les liens vers les correctifs, vous pouvez trouver des discussions intéressantes ...
CL123376 en est un exemple.

couper

Une caractéristique de l' expression de tranche est que x[:] toujours identique à x si le type x est une tranche ou une chaîne. Dans le cas des tranches, cela fonctionne pour tout type d'élément []T

Tout dans la liste ci-dessous est le même ( x - tranche):

• x
• x[:]
• x[:][:]
• ...

unslice trouve des expressions de tranche redondantes similaires. Ces expressions sont nuisibles, tout d'abord, avec une charge cognitive supplémentaire. x[:] a une sémantique assez importante dans le cas de la prise d'une tranche du tableau. Une tranche avec des plages par défaut ne fait que du bruit.

Je demande un patch en CL123375 .

switchTrue

Dans CL123378 , " switch true {...} " est remplacé par " switch {...} ".
Les deux formes sont équivalentes, mais la seconde est plus idiomatique.
Trouvé en vérifiant switchTrue .

La plupart des vérifications stylistiques révèlent précisément de tels cas où les deux options sont acceptables, mais l'une d'entre elles est plus courante et familière à davantage de programmeurs Go. Vérifiez ensuite de la même série.

typeUnparen

Go, comme beaucoup d'autres langages de programmation, aime les parenthèses. À tel point que je suis prêt à en accepter un nombre quelconque:

 type ( t0 int t1 (int) t2 ((int)) // ... ,  . ) 

Mais que se passe-t-il si vous exécutez gofmt ?

 type ( t0 int t1 (int) // <- !   . t2 (int) // <-    ... ) 

C'est pourquoi typeUnparen existe. Il trouve dans le programme toutes les expressions de type dans lesquelles vous pouvez réduire le nombre de crochets. J'ai essayé d'envoyer CL123379 , voyons comment la communauté l'acceptera.

critique en déplacement

Nous n'avons examiné qu'une petite partie des contrôles mis en œuvre. Dans le même temps, leur quantité et leur qualité n'évolueront qu'avec le temps, notamment grâce aux personnes qui ont rejoint le développement .

go-critique est absolument gratuit pour toute utilisation ( licence MIT ), et également ouvert à votre participation au développement du projet. Envoyez-nous des idées de vérifications, vous pouvez immédiatement avec la mise en œuvre, rapporter les bugs et lacunes constatés, partager vos impressions. Vous pouvez également proposer des projets à auditer ou faire rapport sur votre revue de code Go, cette expérience nous est précieuse.

Aller Contribuer Thème

Avez-vous vu l'article Go Contribution Workshop en Russie ? Cet automne sera le deuxième tour. Et cette fois, en plus d'un format et de sponsors plus performants, nous aurons une arme secrète - un merveilleux analyseur statique. Il y aura suffisamment de contributions!

Mais en fait, vous pouvez commencer dès maintenant (bien que ce soit mieux - un peu plus tard, après le gel du code ). Si vous parvenez à vous mettre à l'aise avant le prochain atelier, ce sera très cool, car nous manquons de mentors en Russie.