On Sun, 2023-09-10 at 22:12 -0400, Eric Feng wrote: > On Thu, Sep 7, 2023 at 1:28 PM David Malcolm <dmalc...@redhat.com> > wrote: > > > On Mon, 2023-09-04 at 22:13 -0400, Eric Feng wrote: > >
[...snip...] > > > > > > I know you're emulating the old behavior I implemented way back in > > cpychecker, but I don't like that behavior :( > > > > Specifically, although the patch improves the behavior for symbolic > > values, it regresses the precision of wording for the concrete > > values > > case. If we have e.g. a concrete ob_refcnt of 2, whereas we only > > have > > 1 pointer, then it's more readable to say: > > > > warning: expected ‘obj’ to have reference count: ‘1’ but > > ob_refcnt > > field is ‘2’ > > > > than: > > > > warning: expected ‘obj’ to have reference count: N + ‘1’ but > > ob_refcnt > > field is N + ‘2’ > > > > ...and we shouldn't quote concrete numbers, the message should be: > > > > warning: expected ‘obj’ to have reference count of 1 but > > ob_refcnt field > > is 2 > > > > or better: > > > > warning: ‘*obj’ is pointed to by 1 pointer but 'ob_refcnt' field > > is 2 > > > > > > Can you move the unwrapping of the svalue from the tests below into > > the > > emit vfunc? That way the m_actual_refcnt doesn't have to be a > > constant_svalue; you could have logic in the emit vfunc to print > > readable messages based on what kind of svalue it is. > > > > Rather than 'N', it might be better to say 'initial'; how about: > > > > warning: ‘*obj’ is pointed to by 0 additional pointers but > > 'ob_refcnt' > > field has increased by 1 > > warning: ‘*obj’ is pointed to by 1 additional pointer but > > 'ob_refcnt' > > field has increased by 2 > > warning: ‘*obj’ is pointed to by 1 additional pointer but > > 'ob_refcnt' > > field is unchanged > > warning: ‘*obj’ is pointed to by 2 additional pointers but > > 'ob_refcnt' > > field has decreased by 1 > > warning: ‘*obj’ is pointed to by 1 fewer pointers but 'ob_refcnt' > > field > > is unchanged > > > > and similar? > > > > That makes sense to me as well (indeed I was just emulating the old > behavior)! Will experiment and keep you posted on a revised patch > with this > in mind. This is somewhat of a minor detail but can we emit ‘*obj’ > as > bolded text in the diagnostic message? Currently, I can emit this > (including the asterisk) like so: '*%E'. But unlike using %qE, it > doesn't > bold the body of the single quotations. Is this possible? Yes. You could use %< and %> to get the colorized (and localized) quotes (see pretty-print.cc), but better would probably be to pass a tree for the *obj, rather than obj. You can make this by building a MEM_REF tree node wrapping the pointer (you can see an example of this in the RK_SYMBOLIC case of region_model::get_representative_path_var_1). Dave
