Looks good, please commit!
On Fri, Dec 27, 2013 at 10:39 AM, Will Wilson <[email protected]> wrote: > Hi All, > > Thanks for the great feedback. I've fixed the comment formatting issue, > the spurious typedef for the iterator and integrated Reid's extended test > proposal. > > The other points: > > + if (!FileEnt) >> + FileEnt = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()); >> + >> + if (FileEnt) >> + Includers.push_back(FileEnt); >> Is the second if statement needed? Could we assert(FileEnt); here? > > > FileEnt is NULL in a few test cases so I've kept the conditional for now. > I didn't get chance to investigate further but the behavior should be > unchanged from the previous revision. > > Will, do you think it's worth adding a diagnostic (warning) for this >> new behavior, in the case where the new/MSVC behavior differs from the >> old/GCC behavior? Something like >> warning: in Microsoft mode this #include directive refers to >> 'a/b/foo.h', where >> normal header lookup would have found 'c/foo.h' instead >> [-Wmsvc-include] >> Silently accepting the construct just encourages people to rely on it. >> Giving a diagnostic would at least let the relevant developers know >> that something bogus is going on and that maybe they should fix their >> directory structure. (And if they really don't care, they can silence >> the warning.) > > > An excellent idea. I've implemented what I hope is a reasonable middle > ground between performance and diagnostics. Running the normal lookup rules > just for a (probably ignored) warning seemed a little heavy handed. So in > this case I warn with: > > warning: #include resolved using non-portable MSVC search rules as: > path/to/foo.h [-Wmsvc-include] > > > Let me know what you think and I'll get it in if everyone's happy with it. > > Cheers, > Will. >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
