On Jan 16, 2014, at 2:46 PM, Ben Langmuir <[email protected]> wrote:
> > On Jan 16, 2014, at 12:28 PM, Argyrios Kyrtzidis <[email protected]> wrote: > >> On Jan 16, 2014, at 11:04 AM, Ben Langmuir <[email protected]> wrote: >> >>> On Jan 16, 2014, at 10:15 AM, Argyrios Kyrtzidis <[email protected]> >>> wrote: >>>> >>>> Very nice! I like the import stack in the notes. >>>> Could you also add a note saying something like "please rebuild the >>>> precompiled header ’foo.h.pch’” ? >>> >>> Done. I emitted it conditionally upon Diags.isDiagnosticsInFlight(). I >>> wasn’t sure if that’s desired, but it seems to be the way the other notes >>> are being emitted in this file so I did it too. >> >> (disregard the previous email..) >> >> The test is problematic. It has >> >> -fmodule-map-file=modified-module-dependency.module.map >> >> But with no absolute path it will just try to find it in the current >> directory where the test suite is executing and fail. You later touch it >> (therefore create it) and a second run will pass. >> In general you should avoid relative paths in the test run, if you are going >> to touch something, copy it to the test output directory and use it from >> there. >> >> Also I don’t see any module created in the test’s module cache path. This is >> also indicated by the error message which only contains about the >> precompiled header, no module shows up in the imported stack. >> >> I’d suggest >> - Create a PCH that imports a module which contains a header >> - touch the module header >> - Verify that the error complains about the PCH and that the module shows up >> in the import stack. > > Ah, sorry about the bogus test. The attached includes a rewritten test per > your suggestions. I’ve verified that it passes from a fresh state (and fails > appropriately without the changes from this patch). LGTM!
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
