> On Apr 5, 2017, at 05:44, Alex L <arpha...@gmail.com> wrote: > > > > On 5 April 2017 at 13:38, Duncan Exon Smith <dexonsm...@apple.com > <mailto:dexonsm...@apple.com>> wrote: > > > > On Apr 5, 2017, at 05:13, Aaron Ballman via Phabricator > > <revi...@reviews.llvm.org <mailto:revi...@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. >
SGTM. > > 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 <https://reviews.llvm.org/D30009> > > > > > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
