[cfe-commits] [PATCH] Disallow __has_include and __has_include_next outside of preprocessor directives

Aaron Ballman aaron at aaronballman.com
Tue Jan 15 15:12:38 PST 2013


Yes, I prefer the way that looks -- new patch attached.  Note that it
does change another test in the test cases, but I think the change is
beneficial.

Thanks!

~Aaron

On Tue, Jan 15, 2013 at 5:57 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Tue, Jan 15, 2013 at 5:46 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
>> On Wed, Jan 16, 2013 at 12:37 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>> This patch addresses PR14837, so that __has_include and
>>> __has_include_next do not work outside of preprocessor directives.
>>> This fixes a failing assertion, as well as clarifies the behavior (I
>>> can update the public docs if we think it's desirable).  Patch
>>> Includes test cases.
>>
>> Mechanical issues:
>>
>> +      if (PP.getCurrentLexer()->isParsingPreprocessorDirective())
>>        PP.getCurrentLexer()->LexIncludeFilename(Tok);
>> +      else {
>>
>> Indentation is funny on the "PP" line.
>>
>> +        PP.Diag( SLoc, diag::err_pp_directive_required ) << II->getName();
>>
>> No spaces after "(" and before ")", please.
>
> Good catches.  I'll resolve both.
>
>> Why not reject these cases with error at the very beginning of the
>> function or in the caller, Preprocessor::ExpandBuiltinMacro?
>
> My original thinking was in case it was ill-formed in other ways, but
> that was when I was thinking this would be a warning instead of an
> error. I can certainly move it up if that lends clarity.
>
> Thanks!
>
> ~Aaron
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr14837.patch
Type: application/octet-stream
Size: 3133 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130115/97e4eb0d/attachment.obj>


More information about the cfe-commits mailing list