[cfe-commits] Threadsafety: delayed parsing patch

[Douglas Gregor]

On Aug 24, 2011, at 5:10 PM, Caitlin Sadowski wrote:

> Doug,
> 
> This patch builds off of the previous guarded patch (so as to have
> good test cases) and adds delayed parsing of attributes so as to
> handle cases where we want to, for example, reference a member
> variable in an attribute argument.Comments are appreciated -- this
> patch touches various parts of the parser and so definitely needs to
> be reviewed before committing.

This looks good to me. One general comment about this:

bool Parser::IsThreadSafetyAttribute(llvm::StringRef AttrName){
@@ -696,34 +805,52 @@ bool Parser::IsThreadSafetyAttribute(llvm::StringRef AttrName){
 void Parser::ParseThreadSafetyAttribute(IdentifierInfo &AttrName,
                                         SourceLocation AttrNameLoc,
                                         ParsedAttributes &Attrs,
-                                        SourceLocation *EndLoc) {
-
+                                        LateParsedAttrList *LateAttrs) {
   if (Tok.is(tok::l_paren)) {
-    SourceLocation LeftParenLoc = Tok.getLocation();
-    ConsumeParen(); // ignore the left paren loc for now
+    // store the list of attribute arguments to be parsed
+    // after the class is complete. 
+    if (LateAttrs && !ClassStack.empty()) {
+      LateParsedAttribute *LA = new LateParsedAttribute(this, AttrName,
+                                                        AttrNameLoc);
+      LateAttrs->push_back(LA);
+      getCurrentClass().LateParsedDeclarations.push_back(LA);
+      CachedTokens &Toks = LA->Toks;
+
+      // consume everything up to and including the matching right
+      // parenss
+      ConsumeAndStoreUntil(tok::r_paren, Toks, true, false);

I'd prefer that "LateParsed" be a property that can be specified for an attribute in the TableGen file, and that the late-parsing logic be hoisted out of the thread-safe-attribute parsing logic and into the general attribute-parsing logic. I expect that this would be both cleaner and more general, and it also gives you a great way to test the delayed parsing: just tweak your code to handle *every* attribute with delayed parsing, run the test suite, and see what breaks.

Did you consider this generalization before?

	- Doug