[PATCH] D79895: Add a new warning to warn when passing uninitialized variables as const reference parameters to a function

[Hans Wennborg via Phabricator via cfe-commits]

hans accepted this revision.
hans added a comment.

Very nice! I only have minor comments.

Also I'm curious to see what this will find in Chromium. I guess we'll find out :-)



================
Comment at: clang/include/clang/Analysis/Analyses/UninitializedValues.h:118
+
+  virtual void handleConstRefUseOfUninitVariable(const VarDecl *vd,
+                                                 const UninitUse &use) {}
----------------
nit: May put this after handleUseOfUninitVariable() instead, since it's very similar.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2110
+def warn_uninit_const_reference : Warning<
+  "variable %0 is uninitialized when passed as a const reference parameter "
+  "here">, InGroup<UninitializedConstReference>, DefaultIgnore;
----------------
Minor detail, but I think it should say "argument" instead of "parameter".

My understanding is that parameters are names used when declaring/defining functions, e, g.
int foo(int x) { ... }
here x is a parameter.

And arguments are used when calling functions:
foo(42);
here 42 is an argument.

(https://en.wikipedia.org/wiki/Parameter_(computer_programming))


================
Comment at: clang/lib/Analysis/UninitializedValues.cpp:891
+
+  void handleConstRefUseOfUninitVariable(const VarDecl *vd,
+                                         const UninitUse &use) override {
----------------
I'd probably move this to right after handleUseOfUninitVariable() since they're similar.


================
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:1590
+
+    // flush all const reference uses diags
+    for (const auto &P : constRefUses) {
----------------
nit: capital f and period at the end.


================
Comment at: clang/test/SemaCXX/warn-uninitialized-const-reference.cpp:5
+public:
+    int i;
+    A() {};
----------------
nit: I think 2 spaces for indentation is more common in this code (applies also below).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79895/new/

https://reviews.llvm.org/D79895