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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
