This is an archive of the discontinued LLVM Phabricator instance.

[clang] Fix FIXME in isAlignasAttribute()
ClosedPublic

Authored by rsandifo-arm on Apr 12 2023, 3:06 AM.

Details

Summary

AttributeCommonInfo::isAlignasAttribute() was used in one place:
isCXX11Attribute(). The intention was for isAlignasAttribute()
to return true for the C++ alignas keyword. However, as a FIXME
noted, the function also returned true for the C _Alignas keyword.
This meant that isCXX11Attribute() returned true for _Alignas as
well as for alignas.

AttributeCommonInfos are now always constructed with an
AttributeCommonInfo::Form. We can use that Form to convey whether
a keyword is alignas or not.

The patch uses 1 bit of an 8-bit hole in the current layout
of AttributeCommonInfo. This might not be the best long-term design,
but it should be easy to adapt the layout if necessary (that is,
if other uses are found for the spare bits).

I don't know of a way of testing this (other than grep -c FIXME)

Diff Detail

Event Timeline

rsandifo-arm created this revision.Apr 12 2023, 3:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 12 2023, 3:06 AM
rsandifo-arm requested review of this revision.Apr 12 2023, 3:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 12 2023, 3:06 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

FWIW, the original reason for looking at this was to explore ways of removing AS_AttributeLikeKeyword from https://reviews.llvm.org/D139028 . I wanted to find a way of conveying extra information about a spelling without having to change the existing enums. It seemed like that was essentially the same problem as the one described in the FIXME being fixed here.

erichkeane accepted this revision.Apr 12 2023, 6:43 AM

Huh, this is much less disruptive than I thought it was going to be. LGTM!

This revision is now accepted and ready to land.Apr 12 2023, 6:43 AM
This revision was automatically updated to reflect the committed changes.