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 <[email protected]> wrote: > On Tue, Jan 15, 2013 at 5:46 PM, Dmitri Gribenko <[email protected]> wrote: >> On Wed, Jan 16, 2013 at 12:37 AM, Aaron Ballman <[email protected]> >> 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
pr14837.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
