ahatanak added inline comments.

================
Comment at: include/clang/AST/Decl.h:3250
+  /// This is true if this struct ends with an array marked 'flexible_array'.
+  bool HasFlexibleArrayAttr : 1;
+
----------------
rsmith wrote:
> How is this different from `HasFlexibleArrayMember`? Do we really need both?
As I commented below, we don't need both if we are going to treat an array with 
flexible_array attribute like other existing flexible arrays.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2173
+def err_flexible_array_nested : Error<
+  "struct with a member marked 'flexible_array' cannot be nested">;
 def err_aligned_attribute_argument_not_int : Error<
----------------
rsmith wrote:
> I don't think it's very clear what "nested" means here. I assume you mean 
> that a struct with a `flexible_array` member can't be used as the type of a 
> field. If so, why not? C allows that for other kinds of flexible array 
> members.
Yes, that's what I meant. I disallowed using struct with a flexible_array 
member to be used as a field in another struct just because the use case I had 
in mind didn't need that (see the link below). We don't need to restrict its 
usage if we are going to handle it like other existing flexible array members.

http://lists.llvm.org/pipermail/cfe-dev/2016-February/047561.html


================
Comment at: lib/AST/ExprConstant.cpp:5200
+    } else
       Result.Designator.setInvalid();
     return true;
----------------
rsmith wrote:
> Should we also set `HasFlexibleArrayAttr` on *real* flexible array members 
> (which will be fields with `IncompleteArrayType`) rather than invalidating 
> the designator?
I'm assuming you are also suggesting we call Result.addArray on "real" flexible 
arrays too?


https://reviews.llvm.org/D21453



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

Reply via email to