danielmarjamaki added a comment. In http://reviews.llvm.org/D12359#233152, @sberg wrote:
> causes false positive for > > char * f(char *); > char * g(char * p) { return f(p); } > Sorry for replying this late. This should work in latest patch. ================ 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">, ---------------- aaron.ballman wrote: > > 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. ok thanks for looking it up. I will try to fix all the test cases. ================ 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()); ---------------- aaron.ballman wrote: > > 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. that is not by intention. There is no nonconst use if lhs is a const pointer. I will investigate. ================ Comment at: lib/Parse/ParseExpr.cpp:176 @@ +175,3 @@ + Expr *E = ER.get()->IgnoreParenCasts(); + if (auto *B = dyn_cast<BinaryOperator>(E)) { + if (B->isAssignmentOp()) { ---------------- it's handled better now. ================ Comment at: lib/Parse/ParseExpr.cpp:181 @@ +180,3 @@ + isa<BinaryConditionalOperator>(E) || + isa<ConditionalOperator>(E) || + isa<UnaryOperator>(E)) { ---------------- yes. dontwarn9 was just a test.. if a nonconst pointer is returned then the parameter can't be const. I have corrected the test, see return6 . I will add a new test in the next iteration when a const pointer is returned. ================ 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(); ---------------- aaron.ballman wrote: > > This is called from the Parser only. > > So will this still properly diagnose the same cases from a serialized AST? I don't know what this "serialized AST" is that you are talking about. All I know is the -ast-dump and that flag is only intended as a debugging aid as far as I know. If I just run "clang -cc1 -Wnonconst-parameter somefile.c" then it does not serialize does it? So what flags do I use to serialize etc? I would appreciate if you can show me a command that will cause FP. So I can look at it. I believe that we can't put this in the Sema only. The parser knows better how the result is used and can set "nonconstuse" properly for expressions. For instance I don't want to mark all pointer additions as "nonconstuse" just because some of them are "nonconstuse". http://reviews.llvm.org/D12359 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits