Re: r361997 - [analyzer] print() JSONify: getNodeLabel implementation

2019-05-30 Thread Artem Dergachev via cfe-commits

Hi all,

I'm perfectly fine with reducing this test case to only test the 
`conj_$3\{int, LC3, no stmt, #1\}` part and drop everything else, 
because that's what i've been testing when i originally introduced this 
test.


The lack of determinism here indeed only affects our own self-debugging 
facilities and doesn't affect the end users, while also being a 
characteristic of the most performance-critical low-level part of the 
static analyzer (immutable sets and maps), so while we're aware of the 
problem, we're not actively trying to fix it. Generally we are very 
careful about determinism when it comes to emitting or not emitting 
warnings, but this is not the case here.


On 5/30/19 8:44 AM, Csaba Dabis via cfe-commits wrote:
I have not said there is determinism to print out a map as Static 
Analyzer cannot provide that. We have no plans to make it 
deterministic, as it is just dumping out the ExplodedGraph in JSON 
format, which is an internal stuff.


This non-deterministic behaviour has been seen first only some days 
ago, as no-one ever was that crazy to rewrite the backend of the 
Static Analyzer. Now we are doing that as a part of my GSoC project, 
and that little QoL patch-chain cannot take more than a week of work, 
as we planned.


I have said the non-deterministic behaviour of Windows has been fixed. 
When you put that for-loop to `check-clang` on a Linux machine it will 
pass 100 times, however on Windows it will fail 60 times. I am not a 
Windows user and I did not know about that behaviour to write out a 
map could be that silly. By "hiding" the non-determinism of Windows 
map-handling the Test became deterministic.


We had that structure behind:

First CHECK was:
"constructing_objects": [
  { "location_context": "#0 Call", "calling": "foo", "call_line": 
null, "*items*": [
    { "lctx_id": 1, "stmt_id": 1125, "kind": "construct into local 
variable", "argument_index": null, "pretty": "T t;", "value": "" }

  ]}
]

Second CHECK was:
"constructing_objects": [
  { "location_context": "#0 Call", "calling": "T::T", "call_line": 
"16", "*items*": [
    { "lctx_id": 2, "init_id": 1062, "kind": "construct into member 
variable", "argument_index": null, "pretty": "s", "value": ">s" }

  ]},
  { "location_context": "#1 Call", "calling": "foo", "call_line": 
null, "*items*": [
    { "lctx_id": 1, "stmt_id": 1125, "kind": "construct into local 
variable", "argument_index": null, "pretty": "T t;", "value": "" }

  ]}
]

By removing the `constructing_objects` that Test is deterministic 
every time as no map structure with multiple elements involved.


I am also sad because the core developers did not care about maps. I 
will not too, as I said it is just dumping out the internal of the 
internals.


Thanks for your understanding,
Csaba.

On Thu, May 30, 2019 at 5:28 PM Russell Gallop 
mailto:russell.gal...@gmail.com>> wrote:


Hi Csaba,

I see what Roman means. Output should be deterministic for given
input (unless there is a very good reason not to (e.g. timing or
deliberate randomness)).

You can check whether the output is the same with a script like
below. It looks like the node numbers are different every time. Is
there a good reason for this?

Regards
Russ

$ cat test.sh
#!/bin/bash -xe

python bin/llvm-lit.py -v ../clang/test/Analysis/dump_egraph.cpp
cp tools/clang/test/Analysis/Output/dump_egraph.cpp.tmp.dot ref.dot

for ((i=0;i<100;i++));
do
 python bin/llvm-lit.py -v ../clang/test/Analysis/dump_egraph.cpp
 diff ref.dot tools/clang/test/Analysis/Output/dump_egraph.cpp.tmp.dot
done

On Thu, 30 May 2019 at 16:18, Roman Lebedev mailto:lebedev...@gmail.com>> wrote:

I think we're still talking past each other.

I'm saying that *any* commit that does not fix the underlying
nondeterminizm,
but only hides it by deleting tests that showed that said
determinism
exists in the first place,
is not a fix.

Roman.

On Thu, May 30, 2019 at 6:14 PM Csaba Dabis
mailto:dabis.csab...@gmail.com>> wrote:
>
> Hm, the first `CHECK: constructing_objects` contains only
one element, which is fine,
> the second `CHECK: constructing_objects` has two, which
could be non-determinism,
> but surprisingly it is worked as excepted.
> Because of the edge-case I have changed my mind:
>

https://github.com/llvm/llvm-project/commit/32d545f930ce44614ac8398693dacd1d6dbc41a3
>
> Thanks everyone!
>
> On Thu, May 30, 2019 at 4:52 PM Roman Lebedev
mailto:lebedev...@gmail.com>> wrote:
>>
>> On Thu, May 30, 2019 at 5:48 PM Csaba Dabis via cfe-commits
>> mailto:cfe-commits@lists.llvm.org>> wrote:
>> >
>> > Thanks you!
>> >
>> > Fixed by


Re: r361997 - [analyzer] print() JSONify: getNodeLabel implementation

2019-05-30 Thread Csaba Dabis via cfe-commits
I have not said there is determinism to print out a map as Static Analyzer
cannot provide that. We have no plans to make it deterministic, as it is
just dumping out the ExplodedGraph in JSON format, which is an internal
stuff.

This non-deterministic behaviour has been seen first only some days ago, as
no-one ever was that crazy to rewrite the backend of the Static Analyzer.
Now we are doing that as a part of my GSoC project, and that little QoL
patch-chain cannot take more than a week of work, as we planned.

I have said the non-deterministic behaviour of Windows has been fixed. When
you put that for-loop to `check-clang` on a Linux machine it will pass 100
times, however on Windows it will fail 60 times. I am not a Windows user
and I did not know about that behaviour to write out a map could be that
silly. By "hiding" the non-determinism of Windows map-handling the Test
became deterministic.

We had that structure behind:

First CHECK was:
"constructing_objects": [
  { "location_context": "#0 Call", "calling": "foo", "call_line": null, "
*items*": [
{ "lctx_id": 1, "stmt_id": 1125, "kind": "construct into local
variable", "argument_index": null, "pretty": "T t;", "value": "" }
  ]}
]

Second CHECK was:
"constructing_objects": [
  { "location_context": "#0 Call", "calling": "T::T", "call_line": "16", "
*items*": [
{ "lctx_id": 2, "init_id": 1062, "kind": "construct into member
variable", "argument_index": null, "pretty": "s", "value": ">s" }
  ]},
  { "location_context": "#1 Call", "calling": "foo", "call_line": null, "
*items*": [
{ "lctx_id": 1, "stmt_id": 1125, "kind": "construct into local
variable", "argument_index": null, "pretty": "T t;", "value": "" }
  ]}
]

By removing the `constructing_objects` that Test is deterministic every
time as no map structure with multiple elements involved.

I am also sad because the core developers did not care about maps. I will
not too, as I said it is just dumping out the internal of the internals.

Thanks for your understanding,
Csaba.

On Thu, May 30, 2019 at 5:28 PM Russell Gallop 
wrote:

> Hi Csaba,
>
> I see what Roman means. Output should be deterministic for given input
> (unless there is a very good reason not to (e.g. timing or deliberate
> randomness)).
>
> You can check whether the output is the same with a script like below. It
> looks like the node numbers are different every time. Is there a good
> reason for this?
>
> Regards
> Russ
>
> $ cat test.sh
> #!/bin/bash -xe
>
> python bin/llvm-lit.py -v ../clang/test/Analysis/dump_egraph.cpp
> cp tools/clang/test/Analysis/Output/dump_egraph.cpp.tmp.dot ref.dot
>
> for ((i=0;i<100;i++));
> do
>  python bin/llvm-lit.py -v ../clang/test/Analysis/dump_egraph.cpp
>  diff ref.dot tools/clang/test/Analysis/Output/dump_egraph.cpp.tmp.dot
> done
>
> On Thu, 30 May 2019 at 16:18, Roman Lebedev  wrote:
>
>> I think we're still talking past each other.
>>
>> I'm saying that *any* commit that does not fix the underlying
>> nondeterminizm,
>> but only hides it by deleting tests that showed that said determinism
>> exists in the first place,
>> is not a fix.
>>
>> Roman.
>>
>> On Thu, May 30, 2019 at 6:14 PM Csaba Dabis 
>> wrote:
>> >
>> > Hm, the first `CHECK: constructing_objects` contains only one element,
>> which is fine,
>> > the second `CHECK: constructing_objects` has two, which could be
>> non-determinism,
>> > but surprisingly it is worked as excepted.
>> > Because of the edge-case I have changed my mind:
>> >
>> https://github.com/llvm/llvm-project/commit/32d545f930ce44614ac8398693dacd1d6dbc41a3
>> >
>> > Thanks everyone!
>> >
>> > On Thu, May 30, 2019 at 4:52 PM Roman Lebedev 
>> wrote:
>> >>
>> >> On Thu, May 30, 2019 at 5:48 PM Csaba Dabis via cfe-commits
>> >>  wrote:
>> >> >
>> >> > Thanks you!
>> >> >
>> >> > Fixed by
>> https://github.com/llvm/llvm-project/commit/17604c3486cbe7c27cadac1757cd0a9109a92792
>> >> The non-determinism is still there though, so this isn't correct fix.
>> >>
>> >> > On Thu, May 30, 2019 at 4:16 PM Russell Gallop <
>> russell.gal...@gmail.com> wrote:
>> >> >>
>> >> >> Hi Csaba,
>> >> >>
>> >> >> Failing example attached. Note that the output is different every
>> time so there is potentially more than one failure mode.
>> >> >>
>> >> >> Thanks
>> >> >> Russ
>> >> >>
>> >> >> On Thu, 30 May 2019 at 15:05, Csaba Dabis 
>> wrote:
>> >> >>>
>> >> >>> Hey!
>> >> >>>
>> >> >>> When it fails, could you provide the DOT dump? The path is:
>> llvm-project/build/tools/clang/test/Analysis/Output/dump_egraph.cpp.tmp.dot
>> >> >>>
>> >> >>> Thanks,
>> >> >>> Csaba.
>> >> >>>
>> >> >>> On Thu, May 30, 2019 at 4:00 PM Russell Gallop <
>> russell.gal...@gmail.com> wrote:
>> >> 
>> >>  Hi Csaba,
>> >> 
>> >>  The test dump_egraph.cpp appears to be flaky on Windows. For
>> example here:
>> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/26183
>> .
>> >> 
>> >> 
>> 

Re: r361997 - [analyzer] print() JSONify: getNodeLabel implementation

2019-05-30 Thread Russell Gallop via cfe-commits
Hi Csaba,

I see what Roman means. Output should be deterministic for given input
(unless there is a very good reason not to (e.g. timing or deliberate
randomness)).

You can check whether the output is the same with a script like below. It
looks like the node numbers are different every time. Is there a good
reason for this?

Regards
Russ

$ cat test.sh
#!/bin/bash -xe

python bin/llvm-lit.py -v ../clang/test/Analysis/dump_egraph.cpp
cp tools/clang/test/Analysis/Output/dump_egraph.cpp.tmp.dot ref.dot

for ((i=0;i<100;i++));
do
 python bin/llvm-lit.py -v ../clang/test/Analysis/dump_egraph.cpp
 diff ref.dot tools/clang/test/Analysis/Output/dump_egraph.cpp.tmp.dot
done

On Thu, 30 May 2019 at 16:18, Roman Lebedev  wrote:

> I think we're still talking past each other.
>
> I'm saying that *any* commit that does not fix the underlying
> nondeterminizm,
> but only hides it by deleting tests that showed that said determinism
> exists in the first place,
> is not a fix.
>
> Roman.
>
> On Thu, May 30, 2019 at 6:14 PM Csaba Dabis 
> wrote:
> >
> > Hm, the first `CHECK: constructing_objects` contains only one element,
> which is fine,
> > the second `CHECK: constructing_objects` has two, which could be
> non-determinism,
> > but surprisingly it is worked as excepted.
> > Because of the edge-case I have changed my mind:
> >
> https://github.com/llvm/llvm-project/commit/32d545f930ce44614ac8398693dacd1d6dbc41a3
> >
> > Thanks everyone!
> >
> > On Thu, May 30, 2019 at 4:52 PM Roman Lebedev 
> wrote:
> >>
> >> On Thu, May 30, 2019 at 5:48 PM Csaba Dabis via cfe-commits
> >>  wrote:
> >> >
> >> > Thanks you!
> >> >
> >> > Fixed by
> https://github.com/llvm/llvm-project/commit/17604c3486cbe7c27cadac1757cd0a9109a92792
> >> The non-determinism is still there though, so this isn't correct fix.
> >>
> >> > On Thu, May 30, 2019 at 4:16 PM Russell Gallop <
> russell.gal...@gmail.com> wrote:
> >> >>
> >> >> Hi Csaba,
> >> >>
> >> >> Failing example attached. Note that the output is different every
> time so there is potentially more than one failure mode.
> >> >>
> >> >> Thanks
> >> >> Russ
> >> >>
> >> >> On Thu, 30 May 2019 at 15:05, Csaba Dabis 
> wrote:
> >> >>>
> >> >>> Hey!
> >> >>>
> >> >>> When it fails, could you provide the DOT dump? The path is:
> llvm-project/build/tools/clang/test/Analysis/Output/dump_egraph.cpp.tmp.dot
> >> >>>
> >> >>> Thanks,
> >> >>> Csaba.
> >> >>>
> >> >>> On Thu, May 30, 2019 at 4:00 PM Russell Gallop <
> russell.gal...@gmail.com> wrote:
> >> 
> >>  Hi Csaba,
> >> 
> >>  The test dump_egraph.cpp appears to be flaky on Windows. For
> example here:
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/26183
> .
> >> 
> >> 
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Analysis\dump_egraph.cpp:23:11:
> error: CHECK: expected string not found in input
> >>  // CHECK: \"store\": [\l\{
> \"cluster\": \"t\", \"items\":
> [\l\{ \"kind\":
> \"Default\", \"offset\": 0, \"value\": \"conj_$3\{int, LC3, no stmt, #1\}\"
> >> 
> >>  Running locally, it fails after 2-5 runs for me, running:
> >>  python bin/llvm-lit.py -v ../clang/test/Analysis/dump_egraph.cpp
> >> 
> >>  Please could you take a look?
> >> 
> >>  Note that I'm not certain it was this commit that started the
> flakiness, it is the latest which changed the failing line.
> >> 
> >>  Thanks
> >>  Russ
> >> 
> >>  On Wed, 29 May 2019 at 19:02, Csaba Dabis via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >> >
> >> > Author: charusso
> >> > Date: Wed May 29 11:05:53 2019
> >> > New Revision: 361997
> >> >
> >> > URL: http://llvm.org/viewvc/llvm-project?rev=361997=rev
> >> > Log:
> >> > [analyzer] print() JSONify: getNodeLabel implementation
> >> >
> >> > Summary: This patch also rewrites the ProgramPoint printing.
> >> >
> >> > Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware,
> Szelethus
> >> >
> >> > Reviewed By: NoQ
> >> >
> >> > Subscribers: cfe-commits, szepet, rnkovacs, a.sidorin,
> mikhail.ramalho,
> >> >  donat.nagy, dkrupp
> >> >
> >> > Tags: #clang
> >> >
> >> > Differential Revision: https://reviews.llvm.org/D62346
> >> >
> >> > Modified:
> >> > cfe/trunk/include/clang/Analysis/ProgramPoint.h
> >> > cfe/trunk/lib/Analysis/ProgramPoint.cpp
> >> > cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
> >> > cfe/trunk/test/Analysis/dump_egraph.c
> >> > cfe/trunk/test/Analysis/dump_egraph.cpp
> >> >
> >> > Modified: cfe/trunk/include/clang/Analysis/ProgramPoint.h
> >> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ProgramPoint.h?rev=361997=361996=361997=diff
> >> >
> ==
> >> > --- 

Re: r361997 - [analyzer] print() JSONify: getNodeLabel implementation

2019-05-30 Thread Roman Lebedev via cfe-commits
I think we're still talking past each other.

I'm saying that *any* commit that does not fix the underlying nondeterminizm,
but only hides it by deleting tests that showed that said determinism
exists in the first place,
is not a fix.

Roman.

On Thu, May 30, 2019 at 6:14 PM Csaba Dabis  wrote:
>
> Hm, the first `CHECK: constructing_objects` contains only one element, which 
> is fine,
> the second `CHECK: constructing_objects` has two, which could be 
> non-determinism,
> but surprisingly it is worked as excepted.
> Because of the edge-case I have changed my mind:
> https://github.com/llvm/llvm-project/commit/32d545f930ce44614ac8398693dacd1d6dbc41a3
>
> Thanks everyone!
>
> On Thu, May 30, 2019 at 4:52 PM Roman Lebedev  wrote:
>>
>> On Thu, May 30, 2019 at 5:48 PM Csaba Dabis via cfe-commits
>>  wrote:
>> >
>> > Thanks you!
>> >
>> > Fixed by 
>> > https://github.com/llvm/llvm-project/commit/17604c3486cbe7c27cadac1757cd0a9109a92792
>> The non-determinism is still there though, so this isn't correct fix.
>>
>> > On Thu, May 30, 2019 at 4:16 PM Russell Gallop  
>> > wrote:
>> >>
>> >> Hi Csaba,
>> >>
>> >> Failing example attached. Note that the output is different every time so 
>> >> there is potentially more than one failure mode.
>> >>
>> >> Thanks
>> >> Russ
>> >>
>> >> On Thu, 30 May 2019 at 15:05, Csaba Dabis  wrote:
>> >>>
>> >>> Hey!
>> >>>
>> >>> When it fails, could you provide the DOT dump? The path is: 
>> >>> llvm-project/build/tools/clang/test/Analysis/Output/dump_egraph.cpp.tmp.dot
>> >>>
>> >>> Thanks,
>> >>> Csaba.
>> >>>
>> >>> On Thu, May 30, 2019 at 4:00 PM Russell Gallop 
>> >>>  wrote:
>> 
>>  Hi Csaba,
>> 
>>  The test dump_egraph.cpp appears to be flaky on Windows. For example 
>>  here: 
>>  http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/26183.
>> 
>>  C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Analysis\dump_egraph.cpp:23:11:
>>   error: CHECK: expected string not found in input
>>  // CHECK: \"store\": [\l\{ 
>>  \"cluster\": \"t\", \"items\": 
>>  [\l\{ \"kind\": 
>>  \"Default\", \"offset\": 0, \"value\": \"conj_$3\{int, LC3, no stmt, 
>>  #1\}\"
>> 
>>  Running locally, it fails after 2-5 runs for me, running:
>>  python bin/llvm-lit.py -v ../clang/test/Analysis/dump_egraph.cpp
>> 
>>  Please could you take a look?
>> 
>>  Note that I'm not certain it was this commit that started the 
>>  flakiness, it is the latest which changed the failing line.
>> 
>>  Thanks
>>  Russ
>> 
>>  On Wed, 29 May 2019 at 19:02, Csaba Dabis via cfe-commits 
>>   wrote:
>> >
>> > Author: charusso
>> > Date: Wed May 29 11:05:53 2019
>> > New Revision: 361997
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=361997=rev
>> > Log:
>> > [analyzer] print() JSONify: getNodeLabel implementation
>> >
>> > Summary: This patch also rewrites the ProgramPoint printing.
>> >
>> > Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus
>> >
>> > Reviewed By: NoQ
>> >
>> > Subscribers: cfe-commits, szepet, rnkovacs, a.sidorin, mikhail.ramalho,
>> >  donat.nagy, dkrupp
>> >
>> > Tags: #clang
>> >
>> > Differential Revision: https://reviews.llvm.org/D62346
>> >
>> > Modified:
>> > cfe/trunk/include/clang/Analysis/ProgramPoint.h
>> > cfe/trunk/lib/Analysis/ProgramPoint.cpp
>> > cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
>> > cfe/trunk/test/Analysis/dump_egraph.c
>> > cfe/trunk/test/Analysis/dump_egraph.cpp
>> >
>> > Modified: cfe/trunk/include/clang/Analysis/ProgramPoint.h
>> > URL: 
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ProgramPoint.h?rev=361997=361996=361997=diff
>> > ==
>> > --- cfe/trunk/include/clang/Analysis/ProgramPoint.h (original)
>> > +++ cfe/trunk/include/clang/Analysis/ProgramPoint.h Wed May 29 
>> > 11:05:53 2019
>> > @@ -213,7 +213,7 @@ public:
>> >  ID.AddPointer(getTag());
>> >}
>> >
>> > -  void print(StringRef CR, llvm::raw_ostream ) const;
>> > +  void printJson(llvm::raw_ostream , const char *NL = "\n") const;
>> >
>> >LLVM_DUMP_METHOD void dump() const;
>> >
>> >
>> > Modified: cfe/trunk/lib/Analysis/ProgramPoint.cpp
>> > URL: 
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ProgramPoint.cpp?rev=361997=361996=361997=diff
>> > ==
>> > --- cfe/trunk/lib/Analysis/ProgramPoint.cpp (original)
>> > +++ cfe/trunk/lib/Analysis/ProgramPoint.cpp Wed May 29 11:05:53 2019
>> > @@ -43,151 +43,152 @@ ProgramPoint 

Re: r361997 - [analyzer] print() JSONify: getNodeLabel implementation

2019-05-30 Thread Csaba Dabis via cfe-commits
Hm, the first `CHECK: constructing_objects` contains only one element,
which is fine,
the second `CHECK: constructing_objects` has two, which could be
non-determinism,
but surprisingly it is worked as excepted.
Because of the edge-case I have changed my mind:
https://github.com/llvm/llvm-project/commit/32d545f930ce44614ac8398693dacd1d6dbc41a3

Thanks everyone!

On Thu, May 30, 2019 at 4:52 PM Roman Lebedev  wrote:

> On Thu, May 30, 2019 at 5:48 PM Csaba Dabis via cfe-commits
>  wrote:
> >
> > Thanks you!
> >
> > Fixed by
> https://github.com/llvm/llvm-project/commit/17604c3486cbe7c27cadac1757cd0a9109a92792
> The non-determinism is still there though, so this isn't correct fix.
>
> > On Thu, May 30, 2019 at 4:16 PM Russell Gallop 
> wrote:
> >>
> >> Hi Csaba,
> >>
> >> Failing example attached. Note that the output is different every time
> so there is potentially more than one failure mode.
> >>
> >> Thanks
> >> Russ
> >>
> >> On Thu, 30 May 2019 at 15:05, Csaba Dabis 
> wrote:
> >>>
> >>> Hey!
> >>>
> >>> When it fails, could you provide the DOT dump? The path is:
> llvm-project/build/tools/clang/test/Analysis/Output/dump_egraph.cpp.tmp.dot
> >>>
> >>> Thanks,
> >>> Csaba.
> >>>
> >>> On Thu, May 30, 2019 at 4:00 PM Russell Gallop <
> russell.gal...@gmail.com> wrote:
> 
>  Hi Csaba,
> 
>  The test dump_egraph.cpp appears to be flaky on Windows. For example
> here:
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/26183
> .
> 
> 
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Analysis\dump_egraph.cpp:23:11:
> error: CHECK: expected string not found in input
>  // CHECK: \"store\": [\l\{
> \"cluster\": \"t\", \"items\":
> [\l\{ \"kind\":
> \"Default\", \"offset\": 0, \"value\": \"conj_$3\{int, LC3, no stmt, #1\}\"
> 
>  Running locally, it fails after 2-5 runs for me, running:
>  python bin/llvm-lit.py -v ../clang/test/Analysis/dump_egraph.cpp
> 
>  Please could you take a look?
> 
>  Note that I'm not certain it was this commit that started the
> flakiness, it is the latest which changed the failing line.
> 
>  Thanks
>  Russ
> 
>  On Wed, 29 May 2019 at 19:02, Csaba Dabis via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >
> > Author: charusso
> > Date: Wed May 29 11:05:53 2019
> > New Revision: 361997
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=361997=rev
> > Log:
> > [analyzer] print() JSONify: getNodeLabel implementation
> >
> > Summary: This patch also rewrites the ProgramPoint printing.
> >
> > Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware,
> Szelethus
> >
> > Reviewed By: NoQ
> >
> > Subscribers: cfe-commits, szepet, rnkovacs, a.sidorin,
> mikhail.ramalho,
> >  donat.nagy, dkrupp
> >
> > Tags: #clang
> >
> > Differential Revision: https://reviews.llvm.org/D62346
> >
> > Modified:
> > cfe/trunk/include/clang/Analysis/ProgramPoint.h
> > cfe/trunk/lib/Analysis/ProgramPoint.cpp
> > cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
> > cfe/trunk/test/Analysis/dump_egraph.c
> > cfe/trunk/test/Analysis/dump_egraph.cpp
> >
> > Modified: cfe/trunk/include/clang/Analysis/ProgramPoint.h
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ProgramPoint.h?rev=361997=361996=361997=diff
> >
> ==
> > --- cfe/trunk/include/clang/Analysis/ProgramPoint.h (original)
> > +++ cfe/trunk/include/clang/Analysis/ProgramPoint.h Wed May 29
> 11:05:53 2019
> > @@ -213,7 +213,7 @@ public:
> >  ID.AddPointer(getTag());
> >}
> >
> > -  void print(StringRef CR, llvm::raw_ostream ) const;
> > +  void printJson(llvm::raw_ostream , const char *NL = "\n")
> const;
> >
> >LLVM_DUMP_METHOD void dump() const;
> >
> >
> > Modified: cfe/trunk/lib/Analysis/ProgramPoint.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ProgramPoint.cpp?rev=361997=361996=361997=diff
> >
> ==
> > --- cfe/trunk/lib/Analysis/ProgramPoint.cpp (original)
> > +++ cfe/trunk/lib/Analysis/ProgramPoint.cpp Wed May 29 11:05:53 2019
> > @@ -43,151 +43,152 @@ ProgramPoint ProgramPoint::getProgramPoi
> >  }
> >
> >  LLVM_DUMP_METHOD void ProgramPoint::dump() const {
> > -  return print(/*CR=*/"\n", llvm::errs());
> > +  return printJson(llvm::errs());
> >  }
> >
> > -static void printLocation(raw_ostream , SourceLocation SLoc,
> > -  const SourceManager ,
> > -  StringRef CR,
> > -  StringRef Postfix) {
> > -  if 

Re: r361997 - [analyzer] print() JSONify: getNodeLabel implementation

2019-05-30 Thread Roman Lebedev via cfe-commits
On Thu, May 30, 2019 at 5:48 PM Csaba Dabis via cfe-commits
 wrote:
>
> Thanks you!
>
> Fixed by 
> https://github.com/llvm/llvm-project/commit/17604c3486cbe7c27cadac1757cd0a9109a92792
The non-determinism is still there though, so this isn't correct fix.

> On Thu, May 30, 2019 at 4:16 PM Russell Gallop  
> wrote:
>>
>> Hi Csaba,
>>
>> Failing example attached. Note that the output is different every time so 
>> there is potentially more than one failure mode.
>>
>> Thanks
>> Russ
>>
>> On Thu, 30 May 2019 at 15:05, Csaba Dabis  wrote:
>>>
>>> Hey!
>>>
>>> When it fails, could you provide the DOT dump? The path is: 
>>> llvm-project/build/tools/clang/test/Analysis/Output/dump_egraph.cpp.tmp.dot
>>>
>>> Thanks,
>>> Csaba.
>>>
>>> On Thu, May 30, 2019 at 4:00 PM Russell Gallop  
>>> wrote:

 Hi Csaba,

 The test dump_egraph.cpp appears to be flaky on Windows. For example here: 
 http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/26183.

 C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Analysis\dump_egraph.cpp:23:11:
  error: CHECK: expected string not found in input
 // CHECK: \"store\": [\l\{ 
 \"cluster\": \"t\", \"items\": 
 [\l\{ \"kind\": 
 \"Default\", \"offset\": 0, \"value\": \"conj_$3\{int, LC3, no stmt, #1\}\"

 Running locally, it fails after 2-5 runs for me, running:
 python bin/llvm-lit.py -v ../clang/test/Analysis/dump_egraph.cpp

 Please could you take a look?

 Note that I'm not certain it was this commit that started the flakiness, 
 it is the latest which changed the failing line.

 Thanks
 Russ

 On Wed, 29 May 2019 at 19:02, Csaba Dabis via cfe-commits 
  wrote:
>
> Author: charusso
> Date: Wed May 29 11:05:53 2019
> New Revision: 361997
>
> URL: http://llvm.org/viewvc/llvm-project?rev=361997=rev
> Log:
> [analyzer] print() JSONify: getNodeLabel implementation
>
> Summary: This patch also rewrites the ProgramPoint printing.
>
> Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus
>
> Reviewed By: NoQ
>
> Subscribers: cfe-commits, szepet, rnkovacs, a.sidorin, mikhail.ramalho,
>  donat.nagy, dkrupp
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D62346
>
> Modified:
> cfe/trunk/include/clang/Analysis/ProgramPoint.h
> cfe/trunk/lib/Analysis/ProgramPoint.cpp
> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
> cfe/trunk/test/Analysis/dump_egraph.c
> cfe/trunk/test/Analysis/dump_egraph.cpp
>
> Modified: cfe/trunk/include/clang/Analysis/ProgramPoint.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ProgramPoint.h?rev=361997=361996=361997=diff
> ==
> --- cfe/trunk/include/clang/Analysis/ProgramPoint.h (original)
> +++ cfe/trunk/include/clang/Analysis/ProgramPoint.h Wed May 29 11:05:53 
> 2019
> @@ -213,7 +213,7 @@ public:
>  ID.AddPointer(getTag());
>}
>
> -  void print(StringRef CR, llvm::raw_ostream ) const;
> +  void printJson(llvm::raw_ostream , const char *NL = "\n") const;
>
>LLVM_DUMP_METHOD void dump() const;
>
>
> Modified: cfe/trunk/lib/Analysis/ProgramPoint.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ProgramPoint.cpp?rev=361997=361996=361997=diff
> ==
> --- cfe/trunk/lib/Analysis/ProgramPoint.cpp (original)
> +++ cfe/trunk/lib/Analysis/ProgramPoint.cpp Wed May 29 11:05:53 2019
> @@ -43,151 +43,152 @@ ProgramPoint ProgramPoint::getProgramPoi
>  }
>
>  LLVM_DUMP_METHOD void ProgramPoint::dump() const {
> -  return print(/*CR=*/"\n", llvm::errs());
> +  return printJson(llvm::errs());
>  }
>
> -static void printLocation(raw_ostream , SourceLocation SLoc,
> -  const SourceManager ,
> -  StringRef CR,
> -  StringRef Postfix) {
> -  if (SLoc.isFileID()) {
> -Out << CR << "line=" << SM.getExpansionLineNumber(SLoc)
> -<< " col=" << SM.getExpansionColumnNumber(SLoc) << Postfix;
> +static void printLocation(raw_ostream , SourceLocation Loc,
> +  const SourceManager ) {
> +  Out << "\"location\": ";
> +  if (!Loc.isFileID()) {
> +Out << "null";
> +return;
>}
> +
> +  Out << "{ \"line\": " << SM.getExpansionLineNumber(Loc)
> +  << ", \"column\": " << SM.getExpansionColumnNumber(Loc) << " }";
>  }
>
> -void ProgramPoint::print(StringRef CR, llvm::raw_ostream ) const {
> +void 

Re: r361997 - [analyzer] print() JSONify: getNodeLabel implementation

2019-05-30 Thread Csaba Dabis via cfe-commits
Thanks you!

Fixed by
https://github.com/llvm/llvm-project/commit/17604c3486cbe7c27cadac1757cd0a9109a92792

On Thu, May 30, 2019 at 4:16 PM Russell Gallop 
wrote:

> Hi Csaba,
>
> Failing example attached. Note that the output is different every time so
> there is potentially more than one failure mode.
>
> Thanks
> Russ
>
> On Thu, 30 May 2019 at 15:05, Csaba Dabis  wrote:
>
>> Hey!
>>
>> When it fails, could you provide the DOT dump? The path
>> is: 
>> llvm-project/build/tools/clang/test/Analysis/Output/dump_egraph.cpp.tmp.dot
>>
>> Thanks,
>> Csaba.
>>
>> On Thu, May 30, 2019 at 4:00 PM Russell Gallop 
>> wrote:
>>
>>> Hi Csaba,
>>>
>>> The test dump_egraph.cpp appears to be flaky on Windows. For example
>>> here:
>>> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/26183
>>> .
>>>
>>> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Analysis\dump_egraph.cpp:23:11:
>>> error: CHECK: expected string not found in input
>>> // CHECK: \"store\": [\l\{
>>> \"cluster\": \"t\", \"items\":
>>> [\l\{ \"kind\":
>>> \"Default\", \"offset\": 0, \"value\": \"conj_$3\{int, LC3, no stmt, #1\}\"
>>>
>>> Running locally, it fails after 2-5 runs for me, running:
>>> python bin/llvm-lit.py -v ../clang/test/Analysis/dump_egraph.cpp
>>>
>>> Please could you take a look?
>>>
>>> Note that I'm not certain it was this commit that started the flakiness,
>>> it is the latest which changed the failing line.
>>>
>>> Thanks
>>> Russ
>>>
>>> On Wed, 29 May 2019 at 19:02, Csaba Dabis via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Author: charusso
 Date: Wed May 29 11:05:53 2019
 New Revision: 361997

 URL: http://llvm.org/viewvc/llvm-project?rev=361997=rev
 Log:
 [analyzer] print() JSONify: getNodeLabel implementation

 Summary: This patch also rewrites the ProgramPoint printing.

 Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus

 Reviewed By: NoQ

 Subscribers: cfe-commits, szepet, rnkovacs, a.sidorin, mikhail.ramalho,
  donat.nagy, dkrupp

 Tags: #clang

 Differential Revision: https://reviews.llvm.org/D62346

 Modified:
 cfe/trunk/include/clang/Analysis/ProgramPoint.h
 cfe/trunk/lib/Analysis/ProgramPoint.cpp
 cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
 cfe/trunk/test/Analysis/dump_egraph.c
 cfe/trunk/test/Analysis/dump_egraph.cpp

 Modified: cfe/trunk/include/clang/Analysis/ProgramPoint.h
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ProgramPoint.h?rev=361997=361996=361997=diff

 ==
 --- cfe/trunk/include/clang/Analysis/ProgramPoint.h (original)
 +++ cfe/trunk/include/clang/Analysis/ProgramPoint.h Wed May 29 11:05:53
 2019
 @@ -213,7 +213,7 @@ public:
  ID.AddPointer(getTag());
}

 -  void print(StringRef CR, llvm::raw_ostream ) const;
 +  void printJson(llvm::raw_ostream , const char *NL = "\n") const;

LLVM_DUMP_METHOD void dump() const;


 Modified: cfe/trunk/lib/Analysis/ProgramPoint.cpp
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ProgramPoint.cpp?rev=361997=361996=361997=diff

 ==
 --- cfe/trunk/lib/Analysis/ProgramPoint.cpp (original)
 +++ cfe/trunk/lib/Analysis/ProgramPoint.cpp Wed May 29 11:05:53 2019
 @@ -43,151 +43,152 @@ ProgramPoint ProgramPoint::getProgramPoi
  }

  LLVM_DUMP_METHOD void ProgramPoint::dump() const {
 -  return print(/*CR=*/"\n", llvm::errs());
 +  return printJson(llvm::errs());
  }

 -static void printLocation(raw_ostream , SourceLocation SLoc,
 -  const SourceManager ,
 -  StringRef CR,
 -  StringRef Postfix) {
 -  if (SLoc.isFileID()) {
 -Out << CR << "line=" << SM.getExpansionLineNumber(SLoc)
 -<< " col=" << SM.getExpansionColumnNumber(SLoc) << Postfix;
 +static void printLocation(raw_ostream , SourceLocation Loc,
 +  const SourceManager ) {
 +  Out << "\"location\": ";
 +  if (!Loc.isFileID()) {
 +Out << "null";
 +return;
}
 +
 +  Out << "{ \"line\": " << SM.getExpansionLineNumber(Loc)
 +  << ", \"column\": " << SM.getExpansionColumnNumber(Loc) << " }";
  }

 -void ProgramPoint::print(StringRef CR, llvm::raw_ostream ) const {
 +void ProgramPoint::printJson(llvm::raw_ostream , const char *NL)
 const {
const ASTContext  =
getLocationContext()->getAnalysisDeclContext()->getASTContext();
const SourceManager  = Context.getSourceManager();
 +
 +  

Re: r361997 - [analyzer] print() JSONify: getNodeLabel implementation

2019-05-30 Thread Russell Gallop via cfe-commits
Hi Csaba,

Failing example attached. Note that the output is different every time so
there is potentially more than one failure mode.

Thanks
Russ

On Thu, 30 May 2019 at 15:05, Csaba Dabis  wrote:

> Hey!
>
> When it fails, could you provide the DOT dump? The path
> is: 
> llvm-project/build/tools/clang/test/Analysis/Output/dump_egraph.cpp.tmp.dot
>
> Thanks,
> Csaba.
>
> On Thu, May 30, 2019 at 4:00 PM Russell Gallop 
> wrote:
>
>> Hi Csaba,
>>
>> The test dump_egraph.cpp appears to be flaky on Windows. For example
>> here:
>> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/26183
>> .
>>
>> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Analysis\dump_egraph.cpp:23:11:
>> error: CHECK: expected string not found in input
>> // CHECK: \"store\": [\l\{
>> \"cluster\": \"t\", \"items\":
>> [\l\{ \"kind\":
>> \"Default\", \"offset\": 0, \"value\": \"conj_$3\{int, LC3, no stmt, #1\}\"
>>
>> Running locally, it fails after 2-5 runs for me, running:
>> python bin/llvm-lit.py -v ../clang/test/Analysis/dump_egraph.cpp
>>
>> Please could you take a look?
>>
>> Note that I'm not certain it was this commit that started the flakiness,
>> it is the latest which changed the failing line.
>>
>> Thanks
>> Russ
>>
>> On Wed, 29 May 2019 at 19:02, Csaba Dabis via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: charusso
>>> Date: Wed May 29 11:05:53 2019
>>> New Revision: 361997
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=361997=rev
>>> Log:
>>> [analyzer] print() JSONify: getNodeLabel implementation
>>>
>>> Summary: This patch also rewrites the ProgramPoint printing.
>>>
>>> Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus
>>>
>>> Reviewed By: NoQ
>>>
>>> Subscribers: cfe-commits, szepet, rnkovacs, a.sidorin, mikhail.ramalho,
>>>  donat.nagy, dkrupp
>>>
>>> Tags: #clang
>>>
>>> Differential Revision: https://reviews.llvm.org/D62346
>>>
>>> Modified:
>>> cfe/trunk/include/clang/Analysis/ProgramPoint.h
>>> cfe/trunk/lib/Analysis/ProgramPoint.cpp
>>> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
>>> cfe/trunk/test/Analysis/dump_egraph.c
>>> cfe/trunk/test/Analysis/dump_egraph.cpp
>>>
>>> Modified: cfe/trunk/include/clang/Analysis/ProgramPoint.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ProgramPoint.h?rev=361997=361996=361997=diff
>>>
>>> ==
>>> --- cfe/trunk/include/clang/Analysis/ProgramPoint.h (original)
>>> +++ cfe/trunk/include/clang/Analysis/ProgramPoint.h Wed May 29 11:05:53
>>> 2019
>>> @@ -213,7 +213,7 @@ public:
>>>  ID.AddPointer(getTag());
>>>}
>>>
>>> -  void print(StringRef CR, llvm::raw_ostream ) const;
>>> +  void printJson(llvm::raw_ostream , const char *NL = "\n") const;
>>>
>>>LLVM_DUMP_METHOD void dump() const;
>>>
>>>
>>> Modified: cfe/trunk/lib/Analysis/ProgramPoint.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ProgramPoint.cpp?rev=361997=361996=361997=diff
>>>
>>> ==
>>> --- cfe/trunk/lib/Analysis/ProgramPoint.cpp (original)
>>> +++ cfe/trunk/lib/Analysis/ProgramPoint.cpp Wed May 29 11:05:53 2019
>>> @@ -43,151 +43,152 @@ ProgramPoint ProgramPoint::getProgramPoi
>>>  }
>>>
>>>  LLVM_DUMP_METHOD void ProgramPoint::dump() const {
>>> -  return print(/*CR=*/"\n", llvm::errs());
>>> +  return printJson(llvm::errs());
>>>  }
>>>
>>> -static void printLocation(raw_ostream , SourceLocation SLoc,
>>> -  const SourceManager ,
>>> -  StringRef CR,
>>> -  StringRef Postfix) {
>>> -  if (SLoc.isFileID()) {
>>> -Out << CR << "line=" << SM.getExpansionLineNumber(SLoc)
>>> -<< " col=" << SM.getExpansionColumnNumber(SLoc) << Postfix;
>>> +static void printLocation(raw_ostream , SourceLocation Loc,
>>> +  const SourceManager ) {
>>> +  Out << "\"location\": ";
>>> +  if (!Loc.isFileID()) {
>>> +Out << "null";
>>> +return;
>>>}
>>> +
>>> +  Out << "{ \"line\": " << SM.getExpansionLineNumber(Loc)
>>> +  << ", \"column\": " << SM.getExpansionColumnNumber(Loc) << " }";
>>>  }
>>>
>>> -void ProgramPoint::print(StringRef CR, llvm::raw_ostream ) const {
>>> +void ProgramPoint::printJson(llvm::raw_ostream , const char *NL)
>>> const {
>>>const ASTContext  =
>>>getLocationContext()->getAnalysisDeclContext()->getASTContext();
>>>const SourceManager  = Context.getSourceManager();
>>> +
>>> +  Out << "\"kind\": \"";
>>>switch (getKind()) {
>>>case ProgramPoint::BlockEntranceKind:
>>> -Out << "Block Entrance: B"
>>> +Out << "BlockEntrance\""
>>> +<< ", \"block_id\": "
>>>  << castAs().getBlock()->getBlockID();
>>>  break;
>>>
>>>case 

Re: r361997 - [analyzer] print() JSONify: getNodeLabel implementation

2019-05-30 Thread Csaba Dabis via cfe-commits
Hey!

When it fails, could you provide the DOT dump? The path
is: llvm-project/build/tools/clang/test/Analysis/Output/dump_egraph.cpp.tmp.dot

Thanks,
Csaba.

On Thu, May 30, 2019 at 4:00 PM Russell Gallop 
wrote:

> Hi Csaba,
>
> The test dump_egraph.cpp appears to be flaky on Windows. For example here:
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/26183
> .
>
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Analysis\dump_egraph.cpp:23:11:
> error: CHECK: expected string not found in input
> // CHECK: \"store\": [\l\{
> \"cluster\": \"t\", \"items\":
> [\l\{ \"kind\":
> \"Default\", \"offset\": 0, \"value\": \"conj_$3\{int, LC3, no stmt, #1\}\"
>
> Running locally, it fails after 2-5 runs for me, running:
> python bin/llvm-lit.py -v ../clang/test/Analysis/dump_egraph.cpp
>
> Please could you take a look?
>
> Note that I'm not certain it was this commit that started the flakiness,
> it is the latest which changed the failing line.
>
> Thanks
> Russ
>
> On Wed, 29 May 2019 at 19:02, Csaba Dabis via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: charusso
>> Date: Wed May 29 11:05:53 2019
>> New Revision: 361997
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=361997=rev
>> Log:
>> [analyzer] print() JSONify: getNodeLabel implementation
>>
>> Summary: This patch also rewrites the ProgramPoint printing.
>>
>> Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus
>>
>> Reviewed By: NoQ
>>
>> Subscribers: cfe-commits, szepet, rnkovacs, a.sidorin, mikhail.ramalho,
>>  donat.nagy, dkrupp
>>
>> Tags: #clang
>>
>> Differential Revision: https://reviews.llvm.org/D62346
>>
>> Modified:
>> cfe/trunk/include/clang/Analysis/ProgramPoint.h
>> cfe/trunk/lib/Analysis/ProgramPoint.cpp
>> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
>> cfe/trunk/test/Analysis/dump_egraph.c
>> cfe/trunk/test/Analysis/dump_egraph.cpp
>>
>> Modified: cfe/trunk/include/clang/Analysis/ProgramPoint.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ProgramPoint.h?rev=361997=361996=361997=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Analysis/ProgramPoint.h (original)
>> +++ cfe/trunk/include/clang/Analysis/ProgramPoint.h Wed May 29 11:05:53
>> 2019
>> @@ -213,7 +213,7 @@ public:
>>  ID.AddPointer(getTag());
>>}
>>
>> -  void print(StringRef CR, llvm::raw_ostream ) const;
>> +  void printJson(llvm::raw_ostream , const char *NL = "\n") const;
>>
>>LLVM_DUMP_METHOD void dump() const;
>>
>>
>> Modified: cfe/trunk/lib/Analysis/ProgramPoint.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ProgramPoint.cpp?rev=361997=361996=361997=diff
>>
>> ==
>> --- cfe/trunk/lib/Analysis/ProgramPoint.cpp (original)
>> +++ cfe/trunk/lib/Analysis/ProgramPoint.cpp Wed May 29 11:05:53 2019
>> @@ -43,151 +43,152 @@ ProgramPoint ProgramPoint::getProgramPoi
>>  }
>>
>>  LLVM_DUMP_METHOD void ProgramPoint::dump() const {
>> -  return print(/*CR=*/"\n", llvm::errs());
>> +  return printJson(llvm::errs());
>>  }
>>
>> -static void printLocation(raw_ostream , SourceLocation SLoc,
>> -  const SourceManager ,
>> -  StringRef CR,
>> -  StringRef Postfix) {
>> -  if (SLoc.isFileID()) {
>> -Out << CR << "line=" << SM.getExpansionLineNumber(SLoc)
>> -<< " col=" << SM.getExpansionColumnNumber(SLoc) << Postfix;
>> +static void printLocation(raw_ostream , SourceLocation Loc,
>> +  const SourceManager ) {
>> +  Out << "\"location\": ";
>> +  if (!Loc.isFileID()) {
>> +Out << "null";
>> +return;
>>}
>> +
>> +  Out << "{ \"line\": " << SM.getExpansionLineNumber(Loc)
>> +  << ", \"column\": " << SM.getExpansionColumnNumber(Loc) << " }";
>>  }
>>
>> -void ProgramPoint::print(StringRef CR, llvm::raw_ostream ) const {
>> +void ProgramPoint::printJson(llvm::raw_ostream , const char *NL)
>> const {
>>const ASTContext  =
>>getLocationContext()->getAnalysisDeclContext()->getASTContext();
>>const SourceManager  = Context.getSourceManager();
>> +
>> +  Out << "\"kind\": \"";
>>switch (getKind()) {
>>case ProgramPoint::BlockEntranceKind:
>> -Out << "Block Entrance: B"
>> +Out << "BlockEntrance\""
>> +<< ", \"block_id\": "
>>  << castAs().getBlock()->getBlockID();
>>  break;
>>
>>case ProgramPoint::FunctionExitKind: {
>>  auto FEP = getAs();
>> -Out << "Function Exit: B" << FEP->getBlock()->getBlockID();
>> +Out << "FunctionExit\""
>> +<< ", \"block_id\": " << FEP->getBlock()->getBlockID()
>> +<< ", \"stmt_id\": ";
>> +
>>  if (const ReturnStmt *RS = FEP->getStmt()) {
>> -  Out << CR << " Return: S" << 

Re: r361997 - [analyzer] print() JSONify: getNodeLabel implementation

2019-05-30 Thread Russell Gallop via cfe-commits
Hi Csaba,

The test dump_egraph.cpp appears to be flaky on Windows. For example here:
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/26183
.

C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Analysis\dump_egraph.cpp:23:11:
error: CHECK: expected string not found in input
// CHECK: \"store\": [\l\{ \"cluster\":
\"t\", \"items\": [\l\{
\"kind\": \"Default\", \"offset\": 0, \"value\": \"conj_$3\{int, LC3, no
stmt, #1\}\"

Running locally, it fails after 2-5 runs for me, running:
python bin/llvm-lit.py -v ../clang/test/Analysis/dump_egraph.cpp

Please could you take a look?

Note that I'm not certain it was this commit that started the flakiness, it
is the latest which changed the failing line.

Thanks
Russ

On Wed, 29 May 2019 at 19:02, Csaba Dabis via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: charusso
> Date: Wed May 29 11:05:53 2019
> New Revision: 361997
>
> URL: http://llvm.org/viewvc/llvm-project?rev=361997=rev
> Log:
> [analyzer] print() JSONify: getNodeLabel implementation
>
> Summary: This patch also rewrites the ProgramPoint printing.
>
> Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus
>
> Reviewed By: NoQ
>
> Subscribers: cfe-commits, szepet, rnkovacs, a.sidorin, mikhail.ramalho,
>  donat.nagy, dkrupp
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D62346
>
> Modified:
> cfe/trunk/include/clang/Analysis/ProgramPoint.h
> cfe/trunk/lib/Analysis/ProgramPoint.cpp
> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
> cfe/trunk/test/Analysis/dump_egraph.c
> cfe/trunk/test/Analysis/dump_egraph.cpp
>
> Modified: cfe/trunk/include/clang/Analysis/ProgramPoint.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ProgramPoint.h?rev=361997=361996=361997=diff
>
> ==
> --- cfe/trunk/include/clang/Analysis/ProgramPoint.h (original)
> +++ cfe/trunk/include/clang/Analysis/ProgramPoint.h Wed May 29 11:05:53
> 2019
> @@ -213,7 +213,7 @@ public:
>  ID.AddPointer(getTag());
>}
>
> -  void print(StringRef CR, llvm::raw_ostream ) const;
> +  void printJson(llvm::raw_ostream , const char *NL = "\n") const;
>
>LLVM_DUMP_METHOD void dump() const;
>
>
> Modified: cfe/trunk/lib/Analysis/ProgramPoint.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ProgramPoint.cpp?rev=361997=361996=361997=diff
>
> ==
> --- cfe/trunk/lib/Analysis/ProgramPoint.cpp (original)
> +++ cfe/trunk/lib/Analysis/ProgramPoint.cpp Wed May 29 11:05:53 2019
> @@ -43,151 +43,152 @@ ProgramPoint ProgramPoint::getProgramPoi
>  }
>
>  LLVM_DUMP_METHOD void ProgramPoint::dump() const {
> -  return print(/*CR=*/"\n", llvm::errs());
> +  return printJson(llvm::errs());
>  }
>
> -static void printLocation(raw_ostream , SourceLocation SLoc,
> -  const SourceManager ,
> -  StringRef CR,
> -  StringRef Postfix) {
> -  if (SLoc.isFileID()) {
> -Out << CR << "line=" << SM.getExpansionLineNumber(SLoc)
> -<< " col=" << SM.getExpansionColumnNumber(SLoc) << Postfix;
> +static void printLocation(raw_ostream , SourceLocation Loc,
> +  const SourceManager ) {
> +  Out << "\"location\": ";
> +  if (!Loc.isFileID()) {
> +Out << "null";
> +return;
>}
> +
> +  Out << "{ \"line\": " << SM.getExpansionLineNumber(Loc)
> +  << ", \"column\": " << SM.getExpansionColumnNumber(Loc) << " }";
>  }
>
> -void ProgramPoint::print(StringRef CR, llvm::raw_ostream ) const {
> +void ProgramPoint::printJson(llvm::raw_ostream , const char *NL)
> const {
>const ASTContext  =
>getLocationContext()->getAnalysisDeclContext()->getASTContext();
>const SourceManager  = Context.getSourceManager();
> +
> +  Out << "\"kind\": \"";
>switch (getKind()) {
>case ProgramPoint::BlockEntranceKind:
> -Out << "Block Entrance: B"
> +Out << "BlockEntrance\""
> +<< ", \"block_id\": "
>  << castAs().getBlock()->getBlockID();
>  break;
>
>case ProgramPoint::FunctionExitKind: {
>  auto FEP = getAs();
> -Out << "Function Exit: B" << FEP->getBlock()->getBlockID();
> +Out << "FunctionExit\""
> +<< ", \"block_id\": " << FEP->getBlock()->getBlockID()
> +<< ", \"stmt_id\": ";
> +
>  if (const ReturnStmt *RS = FEP->getStmt()) {
> -  Out << CR << " Return: S" << RS->getID(Context) << CR;
> -  RS->printPretty(Out, /*helper=*/nullptr,
> Context.getPrintingPolicy(),
> -  /*Indentation=*/2, /*NewlineSymbol=*/CR);
> +  Out << RS->getID(Context) << ", \"stmt\": \"";
> +  RS->printPretty(Out, /*Helper=*/nullptr,
> Context.getPrintingPolicy());
> +  Out << '\"';
> +} else {
> +  Out << "null,