[PATCH] D19385: [scan-build] fix logic error warnings emitted on clang code base

John McCall via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 22 13:20:13 PDT 2016


rjmccall added inline comments.

================
Comment at: lib/Frontend/CompilerInstance.cpp:763
@@ -762,8 +762,3 @@
       Includers.push_back(std::make_pair(FindFile, FindFile->getDir()));
-      File = HS->LookupFile(InputFile, SourceLocation(), /*isAngled=*/false,
-                            /*FromDir=*/nullptr,
-                            /*CurDir=*/UnusedCurDir, Includers,
-                            /*SearchPath=*/nullptr,
-                            /*RelativePath=*/nullptr,
-                            /*RequestingModule=*/nullptr,
-                            /*SuggestedModule=*/nullptr, /*SkipCache=*/true);
+      if (HS)
+        File = HS->LookupFile(InputFile, SourceLocation(), /*isAngled=*/false,
----------------
apelete wrote:
> rjmccall wrote:
> > What's the justification for this one?
> CompilerInstance::InitializeSourceManager() could call CompilerInstance::InitializeSourceManager() and pass a null pointer value via the 5th parameter 'HS':
> 
> 718. bool CompilerInstance::InitializeSourceManager(const FrontendInputFile &Input){
> 719.   return InitializeSourceManager(
> 720.       Input, getDiagnostics(), getFileManager(), getSourceManager(),
> 721.       hasPreprocessor() ? &getPreprocessor().getHeaderSearchInfo() : nullptr,
> 722.       getDependencyOutputOpts(), getFrontendOpts());
> 723. }
> 
> In that case, 'HS' object pointer would be null at this point.
> I chose not to assert 'HS' and just check it because we already check if 'File' is assigned a null pointer as the result of HS->LookupFile() below.
> 
> Should it be fixed otherwise ?
I don't really know.

The Sema and AST changes LGTM.


http://reviews.llvm.org/D19385





More information about the cfe-commits mailing list