[PATCH] D18567: Block: Fix a crash when we have type attributes or qualifiers with omitted return type.

[Manman Ren via cfe-commits]

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