This is an archive of the discontinued LLVM Phabricator instance.

[clang] Specify attribute syntax & spelling with a single argument
ClosedPublic

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

Details

Summary

When constructing an attribute, the syntactic form was specified
using two arguments: an attribute-independent syntax type and an
attribute-specific spelling index. This patch replaces them with
a single argument.

In most cases, that's done using a new Form class that combines the
syntax and spelling into a single object. This has the minor benefit
of removing a couple of constructors. But the main purpose is to allow
additional information to be stored as well, beyond just the syntax and
spelling enums.

In the case of the attribute-specific Create and CreateImplicit
functions, the patch instead uses the attribute-specific spelling
enum. This helps to ensure that the syntax and spelling are
consistent with each other and with the Attr.td definition.

If a Create or CreateImplicit caller specified a syntax and
a spelling, the patch drops the syntax argument and keeps the
spelling. If the caller instead specified only a syntax
(so that the spelling was SpellingNotCalculated), the patch
simply drops the syntax argument.

There were two cases of the latter: TargetVersion and Weak.
TargetVersionAttrs were created with GNU syntax, which matches
their definition in Attr.td, but which is also the default.
WeakAttrs were created with Pragma syntax, which does not match
their definition in Attr.td. Dropping the argument switches
them to AS_GNU too (to match [GCC<"weak">]).

Diff Detail

Event Timeline

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

I'm not sold on the value of this patch, but I don't see any problems with it.

Later patches show enough value to this, I'm OK with it. Thanks!

erichkeane accepted this revision.Apr 12 2023, 6:43 AM
This revision is now accepted and ready to land.Apr 12 2023, 6:43 AM

Tweak initializer to avoid an MSVC error: https://godbolt.org/z/sMxsW8G8j