On Mar 6, 2014, at 5:31 PM, Ben Langmuir <[email protected]> wrote:
> > On Mar 6, 2014, at 4:59 PM, Argyrios Kyrtzidis <[email protected]> wrote: > >> >> On Mar 6, 2014, at 4:48 PM, Ben Langmuir <[email protected]> wrote: >> >>> >>> On Mar 5, 2014, at 7:54 PM, Argyrios Kyrtzidis <[email protected]> wrote: >>> >>>> @@ -2014,13 +2074,17 @@ ASTReader::ReadControlBlock(ModuleFile &F, >>>> // files. >>>> unsigned NumInputs = Record[0]; >>>> unsigned NumUserInputs = Record[1]; >>>> - unsigned N = ValidateSystemInputs || >>>> - (HSOpts.ModulesValidateOncePerBuildSession && >>>> - F.Kind == MK_Module) >>>> - ? NumInputs >>>> - : NumUserInputs; >>>> + unsigned N = NumUserInputs; >>>> + if (ValidateSystemInputs || >>>> + (Listener && Listener->needsInputFileVisitation()) || >>>> + (HSOpts.ModulesValidateOncePerBuildSession && F.Kind == >>>> MK_Module)) >>>> + N = NumInputs; >>>> + >>>> for (unsigned I = 0; I < N; ++I) { >>>> InputFile IF = getInputFile(F, I+1, Complain); >>>> + if (const FileEntry *F = IF.getFile()) >>>> + Listener->visitInputFile(F->getName(), I >= NumUserInputs); >>>> if (!IF.getFile() || IF.isOutOfDate()) >>>> return OutOfDate; >>>> } >>>> >>>> The code here combines validation with reporting the dependencies and >>>> things start getting problematic.. >>>> >>>> One issue is that earlier there is this check: >>>> >>>> if (!DisableValidation && >>>> (!HSOpts.ModulesValidateOncePerBuildSession || >>>> F.InputFilesValidationTimestamp <= HSOpts.BuildSessionTimestamp)) { >>>> >>>> so no dependencies are going to get reported if we are skipping validation. >>>> >>>> Another issue is that even if we don't need to report the system >>>> dependencies we still go through all the system headers and stat them to >>>> get an InputFile to pass to the Listener. >>>> >>>> I'd suggest separating the validation block from the "reporting >>>> dependencies". >>>> Also, inside ASTReader::getInputFile() there is the code to get the file >>>> path string from the record ID; factor this out into a separate function >>>> and use that to go through and report the dependencies (to avoid stat'ing). >>>> I believe we always store absolute paths in the module files so this >>>> should work for reporting the dependencies. >>> >>> I’ve gone with this approach. We will end up reading the filename, size, >>> etc. information twice for any file that we are both validating *and* >>> outputting dependency info for. I would like to avoid that, but I didn’t >>> see a nice way to factor the code for that while still keeping the >>> validation and dependency stuff separate. I’m not sure if this overhead >>> matters in practice, and it will only affect clients that validate input >>> files and print dependency info. >> >> As far as building, dependency info will be always requested, but we may >> skip validation during a build session. >> >> But it seems unfortunate that we go through all the system headers even >> though we don't need them, how about changing needsInputFileVisitation() to >> also return whether it needs all files or just the user ones ? >> > > Okay, I’ve done that in the attached patch. It was cleaner to just add a new > function needsSystem…(). I'd probably change the one function to return an enum like "InputFileVisitationKind { None, UserOnly, All }" or something, but WFM!
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
