aaron.ballman added inline comments. ================ Comment at: include/clang/Basic/DiagnosticSemaKinds.td:201 @@ -200,1 +200,3 @@ +def warn_nonconst_parameter : Warning<"parameter %0 can be const">, + InGroup<NonConstParameter>, DefaultIgnore; def warn_unused_variable : Warning<"unused variable %0">, ---------------- > I disagree about this. Normal usage is to enable as much warnings as you can. > > Is it possible for you to show a document, discussion or something that backs > your claim?
Searching through the Clang email archives yields: http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20150504/128373.html http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20140922/115379.html and others as well. This has been the de facto bar for as long as I've been contributing. ================ Comment at: lib/Parse/ParseExpr.cpp:176 @@ +175,3 @@ + if (auto *B = dyn_cast<BinaryOperator>(ER.get())) { + if (B->isAssignmentOp() || B->isAdditiveOp()) { + MarkNonConstUse(B->getLHS()); ---------------- > basic idea is that p can't be const here: ``` void f(int *p) { int *q = p + 1; // ... } ``` But it could be const here: ``` void f(int *p) { const *q = p + 1; } ``` I am not certain that addition, by itself, is sufficient to say the use is non-const. At the least, this could have some comments explaining the rationale with a FIXME. ================ Comment at: lib/Parse/ParseExpr.cpp:181 @@ +180,3 @@ + } else if (isa<CallExpr>(ER.get()) || + isa<ConditionalOperator>(ER.get()) || + isa<UnaryOperator>(ER.get())) { ---------------- >This "conditional expression" check ensures that dontwarn9 does not generate >FP: ``` char *dontwarn9(char *p) { char *x; return p ? p : ""; } ``` This *should* diagnose in C++ because "" is a const char *. ;-) But that's neither here nor there; I think both of your examples suffer from the same analysis issues as mentioned above. Consider: ``` const char *f(char *p) { return p ? p : ""; } char g(char *p) { return *p; } ``` ================ Comment at: lib/Parse/ParseStmt.cpp:376 @@ +375,3 @@ +// Mark symbols in r-value expression as written. +void Parser::MarkNonConstUse(Expr *E) { + E = E->IgnoreParenCasts(); ---------------- > This is called from the Parser only. So will this still properly diagnose the same cases from a serialized AST? http://reviews.llvm.org/D12359 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
