This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] print() JSONify: Checker messages implementation
ClosedPublic

Authored by Charusso on May 17 2019, 2:34 PM.

Diff Detail

Event Timeline

Charusso created this revision.May 17 2019, 2:34 PM
Charusso updated this revision to Diff 201067.May 23 2019, 2:04 PM
  • Improved the JSON format.
  • Now the message is a multi-line string properly JSON-spaced.
NoQ added a comment.May 23 2019, 4:19 PM

Let's add a real test here as well. This code is complicated enough.

Charusso updated this revision to Diff 201162.May 24 2019, 2:29 AM
  • Added a test.

Not sure what causes the nondeterministic CHECKs, but sometimes it writes out
"a->b: moved", "a: moved" instead of the logical order: "a: moved", a->b: moved",
so that it could match the wrong line.

I do not know why we prints out two states at one point in one File-check run.
It looks like that:

"program_state": {
  "store": [
    { "cluster": "a", "items": [
      { "kind": "Default", "offset": 128, "value": "0 S8b" },
      { "kind": "Direct", "offset": 0, "value": "42 S32b" },
      { "kind": "Direct", "offset": 64, "value": "Unknown" },
    ]},
  ],
  "environment": [
    { "location_context": "#0 Call", "calling": "simpleMoveCtorTest", "call_line": null, "items": [
      { "lctx_id": 1008, "stmt_id": 37353, "pretty": "clang_analyzer_printState", "value": "&code{clang_analyzer_printState}" },
    ]},
  ],
  "constraints": null,
  "dynamic_types": null,
  "constructing_objects": null,
  "checker_messages": [
    { "checker": "cplusplus.Move", "messages": [
      "Moved-from objects :", 
      "a: moved", 
      "a->b: moved", 
      "",
    ]},
  ],
}
"program_state": {
  "store": [
    { "cluster": "a", "items": [
      { "kind": "Default", "offset": 128, "value": "0 S8b" },
      { "kind": "Direct", "offset": 0, "value": "42 S32b" },
      { "kind": "Direct", "offset": 64, "value": "Unknown" },
    ]},
  ],
  "environment": [
    { "location_context": "#0 Call", "calling": "simpleMoveCtorTest", "call_line": null, "items": [
      { "lctx_id": 621, "stmt_id": 37353, "pretty": "clang_analyzer_printState", "value": "&code{clang_analyzer_printState}" },
    ]},
  ],
  "constraints": null,
  "dynamic_types": null,
  "constructing_objects": null,
  "checker_messages": [
    { "checker": "cplusplus.Move", "messages": [
      "Moved-from objects :", 
      "a->b: moved", 
      "a: moved", 
      "",
    ]},
  ],
}
NoQ accepted this revision.May 26 2019, 1:50 PM

These prints are different across runs, but same across different states on the same run, right? That's kinda expected because the map is ordered by pointer values internally, which may change across runs. This is not that bad of a non-determinism, i don't think it's worth fixing. Let's test a single line to make sure the test definitely doesn't test the order (i.e., leave only a->b or only a) and then commit!

This revision is now accepted and ready to land.May 26 2019, 1:50 PM
Charusso added a comment.EditedMay 27 2019, 7:15 AM
In D62086#1517701, @NoQ wrote:

These prints are different across runs, but same across different states on the same run, right? That's kinda expected because the map is ordered by pointer values internally, which may change across runs. This is not that bad of a non-determinism, i don't think it's worth fixing. Let's test a single line to make sure the test definitely doesn't test the order (i.e., leave only a->b or only a) and then commit!

So that two printState() dumps are only one test-RUN, which is the new one. Because there are two entries of printState() it is 50% to pass the test as it sometimes catches the other printState() and that is why a->b: moved cannot be set to the first, nor a: moved. It would be cool to allow only one printState() somehow.

NoQ added a comment.May 27 2019, 11:03 AM

I mean, let's remove one of the lines from the test so that it became deterministic and commit and then polish later.

Charusso updated this revision to Diff 201573.EditedMay 27 2019, 1:45 PM
  • 100% pure JSON.
  • Removed one CHECK.

It still looks like broken, but if you like that, I like that too.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 29 2019, 9:01 AM