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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to