[PATCH] D107394: [AIX] "aligned" attribute does not decrease alignment

[Steven Wan via Phabricator via cfe-commits]

stevewan marked an inline comment as done.
stevewan added inline comments.


================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1984
+    auto checkTypeIsTypedef = [](auto &&checkTypeIsTypedef,
+                                 const auto Ty) -> bool {
+      bool isTypedef = false;
----------------
rjmccall wrote:
> Hah.  The knot-tying here is really cute, but I think either just make it a static function somewhere in the file, or make it iterative instead of recursive.
> 
> ...actually, I think it would be better to just make the computation in `getTypeInfo` preserve more information.  Make `TypeInfo` carry a three-valued enum instead of an `AlignIsRequired` flag:
> 
> ```
> enum class AlignRequirement {
>   /// The alignment was not explicit in code.
>   None,
> 
>   /// The alignment comes from an alignment attribute on a typedef.
>   RequiredByTypedef,
> 
>   /// The alignment comes from an alignment attribute on a record type.
>   RequiredByRecord,
> };
> ```
> 
> You can add a method on `TypeInfo` to ask if the alignment is required for any reason and make all the existing clients use that instead.
Good idea. I had thought about a similar approach but was afraid that the change may get too pervasive. I think it turned out fine. I'm now debating whether I should split this whole bool-to-enum change into a separate patch, any preference?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107394/new/

https://reviews.llvm.org/D107394