Oh, that's fair. It'd be good to provide a comment in the test case explaining this (the external use case), since I could imagine someone coming back and thinking this is a bit silly to have for writing internal test cases. (& hopefully we'll avoid proliferation of this feature in internal test cases just by code review)
One thought on the code/design: would it be reasonable to, rather than having the "MatchAllLines" flag, use an invalid ExpectedLoc? You might still need a small-scoped "MatchAllLines" (I'd perhaps rename it to "MatchAnyLine"?) due to the invalid ExpectedLoc used to determine whether to report err_verify_missing_line, but after that you could probably set ExpectedLoc to invalid and use that? On Tue, Jul 8, 2014 at 1:55 AM, Andy Gibbs <[email protected]> wrote: > Hi David, > > This patch is not primarily for clang's test-suite but for making the > "-verify" feature more useful to outside use (it is such a useful feature > already but not yet perfect!). > > We use "-verify", for example, to test our library code. In many test-cases > we test to ensure reasonable (consise, precise) error messages are generated > when the library is used in invalid ways. Therefore, a test-case may have > many "expected-error/warning/note" lines that reference these header files. > However, since the library is under development, the header files can change > and this presents a big problem since every change can have a knock-on > effect on all the test-cases that reference this header file (which means > hundreds of files may need to be updated). > > The supplied patch rectifies this is in (I hope!) a clean way. We've been > using this patch for about a year -- I thought it time to see whether other > users could benefit from it. > > The example in my earlier email using a system header was just a simple > example to demonstrate the change. Perhaps this is a better code snippet, > using instead a library header: > > int(*j)(int) = FunctorTraits<decltype(Lambda2)>::CastToFunctionPointer(); > // expected-error@util/traits/FunctorTraits.h:* {{T does not supply a valid > cast operator}} > // expected-note@-2 {{requested here}} > > Regards, > > Andy > > > ----- Original Message ----- From: "David Blaikie" <[email protected]> > To: "Andy Gibbs" <[email protected]> > Cc: "cfe-commits cfe" <[email protected]> > Sent: Monday, July 07, 2014 10:33 PM > Subject: Re: [PATCH] -verify enhancement to support matching "any" line in > included files > > > >> We generally try not to test against system headers - such a test case >> should just include a local file and then the line number would be >> known. >> >> Is there a particular need to test some behavior against a system header? >> >> On Thu, Jul 3, 2014 at 6:32 AM, Andy Gibbs <[email protected]> >> wrote: >>> >>> Hi, >>> >>> I have made an enhancement to the -verify function of clang to support >>> matching diagnostics in included header files where the line number is >>> not >>> necessarily known (or constant). >>> >>> Currently, matching a diagnostic in an included file is done as in the >>> following fragment: >>> >>> // expected-warning@+2 {{incompatible}} expected-warning@+2 {{literal}} >>> // [email protected]:359 {{here}} >>> printf(12); >>> >>> Problem is, the line number inside stdio.h is dependent on the system, so >>> the attached patch provides support for substituting the line number for >>> '*', as in: >>> >>> // [email protected]:* {{here}} >>> >>> so that the diagnostic is matched without the line number being known. >>> >>> I've intentially limited this feature to line numbers in external files >>> only >>> since the line numbers in the main source file should be always "known". >>> While this change may not have an immediate and particular purpose for >>> the >>> clang test-suite itself, I personally feel it is a good and useful >>> enhancement to -verify for people (like me!) who use this for testing >>> 3rd-party library code. >>> >>> Please let me know if this has support/approval for being committed... >>> >>> Thanks >>> >>> Andy >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>> >> > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
