Page MenuHomePhabricator

Allow applying attributes to subset of allowed subjects.
ClosedPublic

Authored by tra on Thu, Apr 8, 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.Thu, Apr 8, 1:36 PM
tra requested review of this revision.Thu, Apr 8, 1:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptThu, Apr 8, 1:36 PM
tra updated this revision to Diff 336212.Thu, Apr 8, 1:41 PM

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

rsmith added inline comments.Thu, Apr 8, 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.Thu, Apr 8, 2:45 PM

Addressed comments

tra added inline comments.Thu, Apr 8, 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.Thu, Apr 8, 3:19 PM

LGTM

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

LGTM!

This revision was automatically updated to reflect the committed changes.