[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.

Cheers,
Manman


================
Comment at: lib/Sema/SemaType.cpp:1569
@@ +1568,3 @@
+        // Mark them as invalid.
+        attr.setInvalid();
+      }
----------------
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.

================
Comment at: lib/Sema/SemaType.cpp:1609
@@ +1608,3 @@
+    // Warn if we see type qualifiers for omitted return type on a block literal.
+    if (TypeQuals && isOmittedBlockReturnType(declarator)) {
+      diagnoseAndRemoveTypeQualifiers(S, DS, TypeQuals, Result,
----------------
rjmccall wrote:
> Checking TypeQuals again here is redundant.
You are right, I was following the code below this.

================
Comment at: lib/Sema/SemaType.cpp:1611
@@ +1610,3 @@
+      diagnoseAndRemoveTypeQualifiers(S, DS, TypeQuals, Result,
+          DeclSpec::TQ_const | DeclSpec::TQ_volatile | DeclSpec::TQ_atomic,
+          diag::warn_block_literal_qualifiers_on_omitted_return_type);
----------------
rjmccall wrote:
> You're missing at least TQ_restrict.  But why make this an enumerated list at all?
I was trying to reuse diagnoseAndRemoveTypeQualifiers and didn't notice that it does not cover all the qualifiers.
I will rewrite this to not use diagnoseAndRemoveTypeQualifiers.


http://reviews.llvm.org/D18567