On Thu, Aug 31, 2023 at 1:01 PM David Malcolm <dmalc...@redhat.com> wrote:
>
> On Wed, 2023-08-30 at 18:15 -0400, Eric Feng wrote:
> > On Tue, Aug 29, 2023 at 5:14 PM David Malcolm <dmalc...@redhat.com>
> > wrote:
> > >
> > > On Tue, 2023-08-29 at 13:28 -0400, Eric Feng wrote:
> > > > Additionally, by using the old model and the pointer per your
> > > > suggestion,
> > > > we are able to find the representative tree and emit a more
> > > > accurate
> > > > diagnostic!
> > > >
> > > > rc3.c:23:10: warning: expected ‘item’ to have reference count:
> > > > ‘1’
> > > > but ob_refcnt field is: ‘2’
> > > > 23 | return list;
> > > > | ^~~~
> > > > ‘create_py_object’: events 1-4
> > > > |
> > > > | 4 | PyObject* item = PyLong_FromLong(3);
> > > > | | ^~~~~~~~~~~~~~~~~~
> > > > | | |
> > > > | | (1) when ‘PyLong_FromLong’
> > > > succeeds
> > > > | 5 | PyObject* list = PyList_New(1);
> > > > | | ~~~~~~~~~~~~~
> > > > | | |
> > > > | | (2) when ‘PyList_New’ succeeds
> > > > |......
> > > > | 14 | PyList_Append(list, item);
> > > > | | ~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > | | |
> > > > | | (3) when ‘PyList_Append’ succeeds, moving buffer
> > > > |......
> > > > | 23 | return list;
> > > > | | ~~~~
> > > > | | |
> > > > | | (4) here
> > > > |
> > >
> > > Excellent, that's a big improvement.
> > >
> > > >
> > > > If a representative tree is not found, I decided we should just
> > > > bail
> > > > out
> > > > of emitting a diagnostic for now, to avoid confusing the user on
> > > > what
> > > > the problem is.
> > >
> > > Fair enough.
> > >
> > > >
> > > > I've attached the patch for this (on top of the previous one)
> > > > below.
> > > > If
> > > > it also looks good, I can merge it with the last patch and push
> > > > it in
> > > > at
> > > > the same time.
> > >
> > > I don't mind either way, but please can you update the tests so
> > > that we
> > > have some automated test coverage that the correct name is being
> > > printed in the warning.
> > >
> > > Thanks
> > > Dave
> > >
> >
> > Sorry — forgot to hit 'reply all' in the previous e-mail. Resending
> > to
> > preserve our chain on the list:
> >
> > ---
> >
> > Thanks; pushed to trunk with nits fixed:
> > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=597b9ec69bca8acb7a3d65641c0a730de8b27ed4
> > .
>
> Thanks; looks good.
>
> Do you want to add this to the GCC 14 part of the "History" section on
> the wiki page:
> https://gcc.gnu.org/wiki/StaticAnalyzer
> or should I?
Happy to add it myself, but I'm not finding an option to edit the page
(created an account under ef...@gcc.gnu.org). Do I need to be added to
the EditorGroup (https://gcc.gnu.org/wiki/EditorGroup) to do so?
>
> >
> > Incidentally, I updated my formatting settings in VSCode, which I've
> > previously mentioned in passing. In case anyone is interested:
> >
> > "C_Cpp.clang_format_style": "{ BasedOnStyle: GNU, UseTab: Always,
> > TabWidth: 8, IndentWidth: 2, BinPackParameters: false,
> > AlignAfterOpenBracket: Align,
> > AllowAllParametersOfDeclarationOnNextLine: true }",
> >
> > This fixes some issues with the indent width and also ensures
> > function
> > parameters of appropriate length are aligned properly and on a new
> > line each (like the rest of the analyzer code).
>
> Thanks
> Dave
>
>
