[PATCH] D30009: Add support for '#pragma clang attribute'

Alex L via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 5 05:44:19 PDT 2017


On 5 April 2017 at 13:38, Duncan Exon Smith <dexonsmith at apple.com> wrote:

>
>
> > On Apr 5, 2017, at 05:13, Aaron Ballman via Phabricator <
> reviews at reviews.llvm.org> wrote:
> >
> > aaron.ballman added inline comments.
> >
> >
> > ================
> > Comment at: lib/Sema/SemaAttr.cpp:578
> > +    return;
> > +  Diag(PragmaAttributeStack.back().Loc, diag::warn_pragm_attribute_no_
> pop_eof);
> > +}
> > ----------------
> > arphaman wrote:
> >> aaron.ballman wrote:
> >>> Perhaps adding a FixIt here would be a kindness?
> >> Where would the fix-it point to? I think only the user will know the
> location at which they meant to insert `#pragma clang attribute pop`.
> > Given that it's a warning rather than an error, and our recovery
> mechanism is to effectively pop the pragma at the end of the TU, I was
> thinking it could be added at the end of the TU. However, you are correct
> that it's probably not where the programmer needs it,
>
> What about at the end of the file the push is in?  This is likely to be
> used in header files, and it's probably unintentional if it extends past
> the end of the file of the push.
>

Then we'd have to take into account preprocessor directives, as we'd
obviously want to insert it before the '#endif'. Or if the user used
`#pragma once`  then the end of the file could probably work.
IMHO the improvement can be done as a follow-up that improves this and
other pragmas like #pragma pack.


> I think we should consider the same thing for #pragma pack (although
> that's a little off-topic).
>
> > so a FixIt likely isn't appropriate. Does that suggest the warning
> should be an error instead?
>
> Maybe it should be an error, but I still think a fixit would be nice if we
> can find a spot.
>
> >
> >
> > Repository:
> >  rL LLVM
> >
> > https://reviews.llvm.org/D30009
> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170405/acd5db95/attachment.html>


More information about the cfe-commits mailing list