> On Apr 5, 2017, at 05:13, Aaron Ballman via Phabricator 
> <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. 

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
> 
> 
> 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to