Page MenuHomePhabricator

[Sema] Demote call-site-based 'alignment is a power of two' check for AllocAlignAttr into a warning
ClosedPublic

Authored by lebedev.ri on Feb 4 2020, 2:32 PM.

Details

Summary

As @rsmith notes in https://reviews.llvm.org/D73020#inline-672219
while that is certainly UB land, it may not be actually reachable at runtime, e.g.:

template<int N> void *make() {
  if ((N & (N-1)) == 0)
    return operator new(N, std::align_val_t(N));
  else
    return operator new(N);
}
void *p = make<7>();

and we shouldn't really error-out there.

That being said, i'm not really following the logic here.
Which ones of these cases should remain being an error?

Diff Detail

Event Timeline

lebedev.ri updated this revision to Diff 242622.Feb 5 2020, 8:05 AM
lebedev.ri retitled this revision from [Sema] Demote 'alignment is not a power of two' error into a warning to [Sema] Demote call-site-based 'alignment is a power of two' check for AllocAlignAttr into a warning.

Scale ambitions down - only demote the call-site-based AllocAlignAttr error.

erichkeane accepted this revision.Feb 19 2020, 12:29 PM

1 nit, otherwise LGTM.

clang/include/clang/Basic/DiagnosticSemaKinds.td
3037

Can you use the above one (err_alignment_not_power_of_two .Text) so that they are forced to have the same message?

This revision is now accepted and ready to land.Feb 19 2020, 12:29 PM

1 nit, otherwise LGTM.

Thank you for the review!

clang/include/clang/Basic/DiagnosticSemaKinds.td
3037

Oh, i always forget about that feature, thanks for pointing it out!

Rebased, NFC.

This revision was automatically updated to reflect the committed changes.