This is an archive of the discontinued LLVM Phabricator instance.

[clang] handle extended integer constant expressions in _Static_assert (PR #57687)
ClosedPublic

Authored by msebor on Sep 20 2022, 1:46 PM.

Details

Summary

The discussion in PR #57687 turned up an inconsistency in where extended integer constant expressions are accepted: in most contexts Clang accepts boolean expressions involving string literals, it rejects them as the first argument to the _Static_assert expression. This change is my attempt to make _Static_assert consistent with the other contexts and with C++. I looked at how some of the other contexts are handled, hoping to follow their example, but couldn't find a simple way to do it. They all seem quite different from _Static_assert.

Diff Detail

Event Timeline

msebor created this revision.Sep 20 2022, 1:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 20 2022, 1:46 PM
msebor requested review of this revision.Sep 20 2022, 1:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 20 2022, 1:46 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
msebor updated this revision to Diff 461744.Sep 20 2022, 3:57 PM

Update to the latest diff that didn't get picked up in the first submission.

tbaeder added inline comments.
clang/lib/Sema/SemaDeclCXX.cpp
16733

There is Expr::ignoreParenImpCasts() or Expr::ImpCasts() that should do this loop for you.

Thank you for working on this! It looks pretty close to good, with just a few comments. Can you also add a release note for the changes as well (to clang/docs/ReleaseNotes.rst)?

clang/lib/Sema/SemaDeclCXX.cpp
16733

+1, I would use Expr::IgnoreImpCasts() instead of this manual loop.

clang/test/Sema/static-assert.c
79

This makes it a bit easier to see there are two diagnostics expected on that line instead of just one.

msebor updated this revision to Diff 463607.Sep 28 2022, 10:26 AM
msebor marked an inline comment as done.

Changes from previous version:

  • Replace loop with Expr::IgnoreImpCasts().
  • Use a multiline comment in a test to improve readability.
clang/lib/Sema/SemaDeclCXX.cpp
16733

Sure. Thank you both for the suggestion!

aaron.ballman accepted this revision.Sep 28 2022, 11:15 AM

LGTM, though please add the release note when landing.

This revision is now accepted and ready to land.Sep 28 2022, 11:15 AM
This revision was landed with ongoing or failed builds.Sep 28 2022, 12:28 PM
This revision was automatically updated to reflect the committed changes.