This is an archive of the discontinued LLVM Phabricator instance.

[clang][dataflow] Use annotations for optional diagnostic tests
AbandonedPublic

Authored by samestep on Jun 23 2022, 7:45 AM.

Details

Summary

Followup to D127898. This patch makes the optional model tests much nicer by removing the second parameter (list of expected diagnostics) entirely, and instead just checking the set of annotation line numbers against the set of diagnostics (so you only write annotations on exactly the set of lines where you expect a diagnostic).

Currently this just reuses the existing buildStatementToAnnotationMapping which already gets called in checkDataflowHelper, so all annotations are required to be ranges and attached to specific statements. @gribozavr2 originally suggested just letting the annotations exist à la carte and using their line numbers directly, which I think would be a better idea since it would allow us to deal with statements spanning multiple lines. That would take more implementation effort though, so I'm leaving it to a followup.

Diff Detail

Event Timeline

samestep created this revision.Jun 23 2022, 7:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 23 2022, 7:45 AM
samestep requested review of this revision.Jun 23 2022, 7:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 23 2022, 7:45 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
samestep edited the summary of this revision. (Show Details)Jun 23 2022, 7:53 AM
samestep added reviewers: ymandel, sgatev, gribozavr2.
samestep added a subscriber: gribozavr2.
samestep updated this revision to Diff 439415.Jun 23 2022, 8:29 AM
  • Merge branch 'diagnose-api' into diagnose-test-annotations
gribozavr2 accepted this revision.Jun 24 2022, 5:39 AM
gribozavr2 added inline comments.
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
1258
1264–1267

But maybe there's an easy way to do that -- change checkDataflowDiagnosis pass down the llvm::Annotations objects to this callback? You could even make that change in the earlier patch in the stack.

This revision is now accepted and ready to land.Jun 24 2022, 5:39 AM
xazax.hun accepted this revision.Jun 24 2022, 8:40 AM

This looks quite nice. I really like how you solved the problem of diagnostics representation/checking.

ymandel accepted this revision.Jun 27 2022, 9:07 AM