This is an archive of the discontinued LLVM Phabricator instance.

[Sema] Teach -Wcast-align to look at the aligned attribute of the declared variables
ClosedPublic

Authored by ahatanak on Jun 7 2016, 1:33 PM.

Details

Summary

Sema::CheckCastAlign currently ignores the aligned attribute attached to the declared variables, which causes clang to issue spurious warnings. This patch fixes the bug.

Diff Detail

Event Timeline

ahatanak updated this revision to Diff 59942.Jun 7 2016, 1:33 PM
ahatanak retitled this revision from to [Sema] Teach -Wcast-align to look at the aligned attribute of the declared variables.
ahatanak updated this object.
ahatanak added a subscriber: cfe-commits.
ahatanak updated this revision to Diff 78300.Nov 16 2016, 5:55 PM

Rebase and ping.

arphaman added inline comments.
lib/Sema/SemaChecking.cpp
10317

NIT, but I think you don't need the extra variable and the if (SE) below if you extract the code inside if (SE) { into a static function that returns SrcAlign. Then the if/else below can be rewritten like:

if (auto *CE = dyn_cast<CastExpr>(Op)) {
  if (CE->getCastKind() == CK_ArrayToPointerDecay)
    SrcAlign = newFunction(CE->getSubExpr(), Context);
} else if (auto *UO = dyn_cast<UnaryOperator>(Op)) {
  if (UO->getOpcode() == UO_AddrOf)
    SrcAlign = newFunction(UO->getSubExpr(), Context);
}
10328

You can use const auto here.

10330

Same here.

ahatanak updated this revision to Diff 78399.Nov 17 2016, 12:21 PM

Address Alex's review comments.

Define a static function for setting SrcAlign.

arphaman added inline comments.Nov 18 2016, 9:29 AM
lib/Sema/SemaChecking.cpp
10270

I'm not sure if you can, since I don't really know CharUnits, but I think you should just return it instead of passing it as a parameter if it's a POD type (if you will end up doing that you should rename this function as well).

10270

Please add a brief doc comment as well.

ahatanak updated this revision to Diff 78594.Nov 18 2016, 3:55 PM
ahatanak marked 2 inline comments as done.

Make the helper function return the Decl's alignment by value.

arphaman accepted this revision.Nov 28 2016, 2:24 AM
arphaman added a reviewer: arphaman.

LGTM, Thanks

This revision is now accepted and ready to land.Nov 28 2016, 2:24 AM
This revision was automatically updated to reflect the committed changes.