[PATCH] D49210: [CFG] [analyzer] NFC: Enumerate construction context layer kinds and re-use their code for ExprEngine keys.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware.

This is a refactoring patch; no functional change intended.

`ConstructionContext` is composed of `ConstructionContextLayer`s, but the layer information is explicitly discarded when `ConstructionContext` is created, in favor of a more user-friendly and explicit interface for accessing the interesting items in the `ConstructionContext`. Later, however - in the Analyzer, which is currently the only user of this API - there appears `ConstructedObjectKey` which helps tracking path-sensitive information about the constructed object. Apparently, `ConstructedObjectKey` has a lot in common with `ConstructionContextLayer`: it captures a single item that requires attention during object construction.

The common part of these two structures is factored out into a new structure, `ConstructionContextItem`. With that, `ConstructionContextLayer` becomes a wrapper around `ConstructionContextItem` that additionally temporarily organizes the items as a linked list, and `ConstructedObjectKey` becomes a wrapper around `ConstructionContextItem`that additionally carries path-sensitive stack frame information.

Once `ConstructionContextItem` is factored out, i also made it a bit more explicit when it comes to what sort of information does it carry. It now contains an enumeration of item kinds ("variable", "destructor", "materialization", "elidable copy"). Such clarification was necessary for supporting argument constructors because otherwise it's hard to discriminate between an elidable copy layer (identified by a construct-expression and, occasionally, index 0) and a layer for the first argument of a constructor (identified by a construct-expression and, intentionally, index 0). I believe it was a good idea in general to be more explicit about what the layer means, rather than give its clients an ad-hoc pile of raw data to work with. It also allows fancy switches in `createFromLayers()` that check that all cases were handled, and many assertions become more explicit.

I did not, however, want to introduce a full-scale class hierarchy of `ConstructionContextItem`s because it'd result in a ridiculous amount of boilerplate. Having them easy to construct via implicit conversion, on the other hand, seems very user-friendly and i'm not seeing much error surface.

One of the questionable ideas here is to use a special item kind `ElidedDestructor` that is only used by the analyzer (to track destructors that correspond to elided constructors) and is never provided by the CFG (so unlike other items it has only one use). This allows us to stop using an extra program state set for elided destructors, but i'm not sure it was a beautiful thing to pursue.


Repository:
  rC Clang

https://reviews.llvm.org/D49210

Files:
  include/clang/Analysis/ConstructionContext.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/Analysis/CFG.cpp
  lib/Analysis/ConstructionContext.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D49210.155092.patch
Type: text/x-patch
Size: 42171 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180712/a4c3dfb0/attachment-0001.bin>