[cfe-dev] Attribute parsing at end of translation unit?

[Douglas Gregor]

On Dec 10, 2012, at 3:07 PM, Delesley Hutchins <delesley at google.com> wrote:

> I am considering changing the way in which thread safety attributes
> are parsed, but I wanted to get some feedback.
> 
> Thread safety analysis currently parses attributes at the end of the
> class where they occur, just like method bodies.  However, this
> strategy can lead to situations where there is no valid way to
> annotate a piece of code.  Here's a toy example:
> 
> 
> class Graph;
> 
> class Node {
> public:
>  // error: Graph has incomplete type.
>  void reachable(Graph* g, int nodeID) SHARED_LOCKS_REQUIRED(g->mu_);
> 
> private:
>  // error: Graph has incomplete type.
>  std::vector<int> edges_ GUARDED_BY(&Graph::mu_);
> };
> 
> 
> class Graph {
>  friend class Node;
> 
>  Mutex mu_;
>  std::vector<Node> nodes_ GUARDED_BY(mu_);
> };
> 
> 
> There's a circular dependency here between Graph and Node.  You could
> try to resolve the dependency by refactoring, but the usual ways of
> breaking dependencies in C++ don't work with attributes.  The body of
> a method can be defined outside of its class, but the attribute can't;
> there's no way to redeclare a method (or a data member) with a new
> attribute.  Moreover, refactoring is not always feasible.  When
> annotating an existing code base, the cost of refactoring is high.
> And given a set of tightly-coupled classes, where annotations on one
> class must refer to mutexes in the others, there may be no good way to
> unravel the dependencies at all.
> 
> I am considering moving attribute parsing to the end of the
> translation unit.  However, this has its own set of disadvantages:
> 
> (1) The lexical scope of an attribute would not be obvious from the
> source code, so there might be occasional unexpected behavior with
> name resolution, etc.
> 
> (2) I'm not sure how template instantiation of attributes would work.
> 
> (3) There may be some performance impact from delaying parsing for that long.
> 
> (4) Thread safety analysis would also have to be deferred until the
> end of the translation unit, with additional performance and memory
> impact (it's part of AnalysisBasedWarnings).
> 
> Together, these disadvantages may outweigh the advantage of having a
> more flexible attribute language.  I'm interested in hearing opinions,
> comments, and/or suggestions on the matter.  :-)


I don't have an alternative implementation for you, but personally I feel that these disadvantages greatly outweigh the advantages of a more flexible attribute language. Delaying all of the diagnostics associated with an attribute is really, really weird *unless* you're also talking about moving those diagnostics into some separate tool or static-analyzer checker.

	- Doug