danielmarjamaki added a comment.
> > > The checker isn't currently path sensitive as it doesn't pay attention
>
> >
>
> > > to control flow graphs or how pointer values flow through a function
>
> >
>
> > > body. I suppose this is a matter of scope more than anything; I see a
>
> >
>
> > > logical extension of this functionality being with local variables as
>
> >
>
> > > well as parameters. So, for instance:
>
> >
>
> > >
>
> >
>
> > > void f(int *p) {
>
> >
>
> > >
>
> >
>
> > > int *i = p;
>
> >
>
> > > std::cout << *i;
>
> >
>
> > >
>
> >
>
> > > }
>
> >
>
> >
>
> > Imho that path analysis is not very interesting. The "p" can't be const
> > until we see that "i" is const.
>
>
> Correct, but from the user's perspective, why are we not telling them
> both can be const?
We want to have simple and clear warning messages. I will currently just write
"parameter p can be const." Imho that is simple and clear. In your example I
believe it would be required with a more complex message. Because p can't be
const. It can only be const if i is made const first.
As I see it "my" analysis does not have any false negatives that would be
avoided. It's just that 2 separate simple messages are clumped together into 1
more complex message.
I also believe that solving this iteratively in small steps is less error prone.
> > if we talk about the user interface.. imho it would be nice that this is a
> > compiler warning. the analysis is quick and there should be little noise.
>
>
> I'm not certain about the performance of the analysis (I suspect it's
> relatively cheap though), but we do not usually want off-by-default
> warnings in the frontend, and I suspect that this analysis would have
> to be off by default due to the chattiness on well-formed code.
hmm.. I believe that this is common practice. I can see that people want to
turn it off for legacy code though.
but we can look at the warnings on real code and discuss those. I have results
from Clang.
================
Comment at: lib/Sema/SemaDecl.cpp:10334
@@ +10333,3 @@
+ continue;
+ // To start with we don't warn about structs.
+ if (T->getPointeeType().getTypePtr()->isRecordType())
----------------
aaron.ballman wrote:
> This seems *really* limiting (especially for C++ code), why the restriction?
2 reasons...
1. I don't think we should warn for structs whenever it's possible to make them
const.
Example code:
struct FRED { char *str; };
void f(struct FRED *fred) {
strcpy(fred->str, "abc");
}
fred can be const but imho we should not warn about this. Imho it would be
misleading to make fred const.
2. I wanted to keep the scope of this checker limited to start with. If we want
to warn about structs also I need to write much more code in the MarkWritten
etc.
http://reviews.llvm.org/D12359
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
