This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Add a prunable note for skipping virtual base initializers in subclasses.
ClosedPublic

Authored by NoQ on May 10 2019, 8:19 PM.

Details

Summary

When initialization of base classes is skipped as in D61816, we might as well tell the user about it, because this aspect of C++ isn't very well-known.

I used the new note tags feature (D58367) in order to implement it. In order to make use of it, i had to allow note tags to produce prunable notes, and i also moved the note tag factory to CoreEngine.

Diff Detail

Repository
rC Clang

Event Timeline

NoQ created this revision.May 10 2019, 8:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 10 2019, 8:19 PM
NoQ updated this revision to Diff 199121.May 10 2019, 8:21 PM

Fix formatting in tests.

NoQ added a comment.May 10 2019, 8:25 PM

This is how it looks:

NoQ updated this revision to Diff 199351.May 13 2019, 6:00 PM

Improve note text so that to avoid the singular/plural problem.

Charusso accepted this revision.May 14 2019, 1:11 PM

What about "the most derived class" or "a superclass" instead of "the superclass"? (https://en.cppreference.com/w/cpp/language/derived_class)
May the sentence is a little bit too long. It would be cool to say "by A" or something more simple and precise.

clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
223 ↗(On Diff #199351)

I think it is better to reduce the number of ifs and merge the related comment as it emphasizes there is only one thing happen.

This revision is now accepted and ready to land.May 14 2019, 1:11 PM
NoQ updated this revision to Diff 199534.May 14 2019, 4:36 PM
NoQ marked an inline comment as done.

What about "the most derived class"

Sounds great!

It would be cool to say "by A" or something more simple and precise.

We can't do that in all cases (we don't necessarily see the constructor of the most derived class) but in some cases we might be able to do it (when we're sure that we actually see it). Added a TODO.

Charusso accepted this revision.May 14 2019, 9:42 PM
NoQ updated this revision to Diff 201359.May 24 2019, 4:12 PM

Rebase.

This revision was automatically updated to reflect the committed changes.