Page MenuHomePhabricator

Allow applying attributes to subset of allowed subjects.
ClosedPublic

Authored by tra on Apr 8 2021, 1:36 PM.

Details

Summary

The original intent of enforcing exact match between apply_to and the attribute's was to
ensure that the user will know what declarations receive the attribute. If the compiler changes the set of allowed attributes in the future.
https://reviews.llvm.org/D30009?id=88764#inline-260744

However, the exact match, as implemented now, is too conservative. E.g. it does not allow using apply_to=variables(is_global) with an attribute which has SubjectList<[Var]>.
Applying an attribute to a subset of the allowed subjects should also be allowed.

Diff Detail

Event Timeline

tra created this revision.Apr 8 2021, 1:36 PM
tra requested review of this revision.Apr 8 2021, 1:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 8 2021, 1:36 PM
tra updated this revision to Diff 336212.Apr 8 2021, 1:41 PM

Updated Sema/pragma-attribute-strict-subjects.c

rsmith added inline comments.Apr 8 2021, 2:13 PM
clang/lib/Sema/SemaAttr.cpp
896

Does this need to be a loop? (Can we have a grandparent matcher rule?)

clang/test/Sema/pragma-attribute-strict-subjects.c
59

The FIXME here is "fixed" now. Please can you add another example that shows the incorrect comma and move the FIXME there?

tra updated this revision to Diff 336236.Apr 8 2021, 2:45 PM

Addressed comments

tra added inline comments.Apr 8 2021, 2:46 PM
clang/lib/Sema/SemaAttr.cpp
896

No. We currently have two levels of tablegen classes: AttrSubjectMatcherRule<list<...AttrSubjectMatcherSubRule> subrules> and AttrSubjectMatcherSubRule does not allow further nesting.

clang/test/Sema/pragma-attribute-strict-subjects.c
59

Added another matcher to preserve the comma in the diagnostic.

rsmith accepted this revision.Apr 8 2021, 3:19 PM

LGTM

This revision is now accepted and ready to land.Apr 8 2021, 3:19 PM
aaron.ballman accepted this revision.Apr 9 2021, 3:43 AM
aaron.ballman added a subscriber: aaron.ballman.

LGTM!

This revision was automatically updated to reflect the committed changes.