manmanren added a comment. Thanks for reviewing!
I will update the patch addressing the comments soon! Manman ================ Comment at: lib/Sema/SemaType.cpp:1569 @@ +1568,3 @@ + // Mark them as invalid. + attr.setInvalid(); + } ---------------- rsmith wrote: > rjmccall wrote: > > manmanren wrote: > > > rjmccall wrote: > > > > It's not generally a good idea to set things as invalid if you're just > > > > emitting a warning. It might be different for parsed AttributeList > > > > objects, but... I'm not sure about that. > > > Here we mark the AttributeList as invalid so when we call > > > processTypeAttrs later, we will ignore these attributes, instead of > > > creating AttributedType based on DependentTy for omitted block return > > > type. > > I'm just worried that there might be code which suppresses *error* > > diagnostics (or semantic analysis) when it sees an invalid attribute. Like > > I said, though, that might not be a problem for AttributeList. > Even if it didn't cause problems today, it's very likely to cause confusion > and problems down the line. We should not have two different meanings for > "invalid" for different kinds of AST nodes. Manman, can you remove the > attribute(s) in question from the attribute list instead? John and Richard, I got your point now. I didn't think about removing it from the attribute list before. I looked at the interface for AttributeList, there is no interface for removing an item from the list, it has a setNext function. But I found a helper function called spliceAttrOutOfList, I will try to use it. http://reviews.llvm.org/D18567 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
