[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

This revision was automatically updated to reflect the committed changes. Closed by commit rL365985: [Sema] Diagnose default-initialization, destruction, and copying of (authored by ahatanak, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63753/new/ https://reviews.llvm.org/D63753 ___

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak updated this revision to Diff 209626. ahatanak added a comment. In Type.h, move method declarations down and mention that the predicates imply the associated basic non-triviality predicates. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63753/new/

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak updated this revision to Diff 209622. ahatanak marked 3 inline comments as done. ahatanak added a comment. Improve comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63753/new/ https://reviews.llvm.org/D63753 Files: include/clang/AST/Decl.h

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

rjmccall added a comment. Thanks, just a few minor comment requests now. Comment at: include/clang/AST/DeclBase.h:1453 +/// copy. +uint64_t HasNonTrivialToPrimitiveCopyCUnion : 1; + Please include in these comments that these imply the associated basic

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak added inline comments. Comment at: lib/Sema/SemaDecl.cpp:12053 +NTCUC_UninitAutoVar); } + rjmccall wrote: > ahatanak wrote: > > rjmccall wrote: > > > Please add a comment explaining why this is specific to local

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak updated this revision to Diff 209565. ahatanak marked an inline comment as done. ahatanak added a comment. Diagnose C union globals that are non-trivial to default-initialize. Add 3 bits to RecordDeclBitfields for default-initialize, destruct, and copy to fast-path checking.

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

rjmccall added inline comments. Comment at: lib/Sema/SemaDecl.cpp:12053 +NTCUC_UninitAutoVar); } + ahatanak wrote: > rjmccall wrote: > > Please add a comment explaining why this is specific to local variables. > I was trying to

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak marked 2 inline comments as done. ahatanak added inline comments. Comment at: lib/Sema/SemaDecl.cpp:12053 +NTCUC_UninitAutoVar); } + rjmccall wrote: > Please add a comment explaining why this is specific to local

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak updated this revision to Diff 209108. ahatanak marked 3 inline comments as done. ahatanak added a comment. Address review comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63753/new/ https://reviews.llvm.org/D63753 Files:

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

rjmccall added inline comments. Comment at: lib/Sema/SemaExpr.cpp:6097 + // non-trivial to copy or default-initialize. + checkNonTrivialCUnionInInitList(ILE); + } ahatanak wrote: > rjmccall wrote: > > Can we extract a common function that checks the

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak marked 3 inline comments as done. ahatanak added inline comments. Comment at: lib/Sema/SemaExpr.cpp:6097 + // non-trivial to copy or default-initialize. + checkNonTrivialCUnionInInitList(ILE); + } rjmccall wrote: > Can we extract a common

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

rjmccall added a comment. Thanks, that looks great. A few more requests and then this will be ready to go, I think. Comment at: include/clang/AST/DeclBase.h:1454 /// Number of non-inherited bits in RecordDeclBitfields. enum { NumRecordDeclBits = 11 };

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak updated this revision to Diff 208861. ahatanak marked 2 inline comments as done. ahatanak added a comment. Add a bit to `RecordDeclBitfields` that indicates whether the record has a non-trivial C union member. Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

rjmccall added inline comments. Comment at: include/clang/AST/Type.h:1133 + /// Check if this is or contains a non-trivial C struct/union type. + bool hasNonTrivialPrimitiveCStruct() const; ahatanak wrote: > rjmccall wrote: > > You only want these checks to

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak marked an inline comment as done. ahatanak added inline comments. Comment at: include/clang/AST/Type.h:1133 + /// Check if this is or contains a non-trivial C struct/union type. + bool hasNonTrivialPrimitiveCStruct() const; rjmccall wrote: > You

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

rjmccall added inline comments. Comment at: include/clang/AST/Type.h:1133 + /// Check if this is or contains a non-trivial C struct/union type. + bool hasNonTrivialPrimitiveCStruct() const; You only want these checks to trigger on unions with non-trivial

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak marked an inline comment as done. ahatanak added inline comments. Comment at: lib/Sema/SemaExpr.cpp:16218 +checkNonTrivialCUnion(E->getType(), E->getExprLoc(), + Sema::NTCUC_LValueToRValueVolatile); + ahatanak wrote: >

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak marked an inline comment as done. ahatanak added inline comments. Comment at: lib/Sema/SemaExpr.cpp:16218 +checkNonTrivialCUnion(E->getType(), E->getExprLoc(), + Sema::NTCUC_LValueToRValueVolatile); + rjmccall wrote: >

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak updated this revision to Diff 207600. ahatanak added a comment. Call `hasNonTrivialPrimitiveCStruct` to check whether the type is a non-trivial C struct/union before calling `checkNonTrivialCUnion`. Fix comments. Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

rjmccall added inline comments. Comment at: lib/Sema/SemaExpr.cpp:16218 +checkNonTrivialCUnion(E->getType(), E->getExprLoc(), + Sema::NTCUC_LValueToRValueVolatile); + ahatanak wrote: > rjmccall wrote: > > ahatanak wrote: > > >

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak marked an inline comment as done. ahatanak added inline comments. Comment at: lib/Sema/SemaExpr.cpp:16218 +checkNonTrivialCUnion(E->getType(), E->getExprLoc(), + Sema::NTCUC_LValueToRValueVolatile); + rjmccall wrote: >

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

rjmccall added inline comments. Comment at: lib/Sema/SemaExpr.cpp:16218 +checkNonTrivialCUnion(E->getType(), E->getExprLoc(), + Sema::NTCUC_LValueToRValueVolatile); + ahatanak wrote: > rjmccall wrote: > > ahatanak wrote: > > >

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak marked an inline comment as done. ahatanak added inline comments. Comment at: lib/Sema/SemaExpr.cpp:16218 +checkNonTrivialCUnion(E->getType(), E->getExprLoc(), + Sema::NTCUC_LValueToRValueVolatile); + rjmccall wrote: >

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

rjmccall added inline comments. Comment at: lib/Sema/SemaExpr.cpp:16218 +checkNonTrivialCUnion(E->getType(), E->getExprLoc(), + Sema::NTCUC_LValueToRValueVolatile); + ahatanak wrote: > rjmccall wrote: > > It looks like you're

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak marked an inline comment as done. ahatanak added inline comments. Comment at: lib/Sema/SemaExpr.cpp:16218 +checkNonTrivialCUnion(E->getType(), E->getExprLoc(), + Sema::NTCUC_LValueToRValueVolatile); + rjmccall wrote: > It

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

rjmccall added inline comments. Comment at: include/clang/Sema/Sema.h:2124 +// Implicitly initialized subobject. +NTCUC_ImplicitInitSubObject, +// Uninialized variable with automatic storage duration. "default-initialized", please.

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:633 + "capture a variable of type %1}3 " + "since it %select{contains a union that |}2is non-trivial to " + "%select{default-initialize|destruct|copy}0">; rjmccall wrote: >

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak updated this revision to Diff 206995. ahatanak marked 7 inline comments as done. ahatanak added a comment. Address review comments. Diagnose lvalue-to-rvalue conversion of volatile non-trivial C types. Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

rjmccall added inline comments. Comment at: lib/Sema/SemaDecl.cpp:11085 +if (isa(I)) + continue; +if (auto E = dyn_cast(I)) ahatanak wrote: > rjmccall wrote: > > Why is this okay? Don't we need to check default-initialization for these? > I didn't

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak marked 2 inline comments as done. ahatanak added inline comments. Comment at: include/clang/Sema/Sema.h:2126 +NTCUC_AutoVar, +// Initializer expression for object with automatic storage duration. +NTCUC_AutoObjInit, rjmccall wrote: > Please

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

rjmccall added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:633 + "capture a variable of type %1}3 " + "since it %select{contains a union that |}2is non-trivial to " + "%select{default-initialize|destruct|copy}0">; Should this be

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak planned changes to this revision. ahatanak added a comment. This patch should diagnose lvalue-to-rvalue conversion of volatile non-trivial C unions too, but it currently doesn't. void test(volatile union U0 *a) { (void)*a; } Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

ahatanak created this revision. ahatanak added reviewers: rjmccall, rsmith. ahatanak added a project: clang. Herald added subscribers: dexonsmith, jkorous. This patch diagnoses uses of non-trivial C unions in the following contexts: - function parameters - function returns - variables with