Fix false positives in -Wmsvc-include by continuing header search

[rnk]

Summary

This makes Clang and LLVM -Wmsvc-include clean.

I believe the correct behavior here is to avoid updating the cache when
we find the header via MSVC's search rules.

[rnk]

Closed by commit rL201615 (authored by @rnk).

cfe/trunk/lib/Lex/HeaderSearch.cpp

void HeaderSearch::setTarget(const TargetInfo &Target) {
   ModMap.setTarget(Target);
 }
 
 
 //===----------------------------------------------------------------------===//
 // Header File Location.
 //===----------------------------------------------------------------------===//
 
+/// \brief Return true with a diagnostic if the file that MSVC would have found
+/// fails to match the one that Clang would have found with MSVC header search
+/// disabled.
+static bool checkMSVCHeaderSearch(DiagnosticsEngine &Diags,
+                                  const FileEntry *MSFE, const FileEntry *FE,
+                                  SourceLocation IncludeLoc) {
+  if (MSFE && FE != MSFE) {
+    Diags.Report(IncludeLoc, diag::ext_pp_include_search_ms) << MSFE->getName();
+    return true;
+  }
+  return false;
+}
 
 /// LookupFile - Given a "foo" or \<foo> reference, look up the indicated file,
 /// return null on failure.  isAngled indicates whether the file reference is
 /// for system \#include's or not (i.e. using <> instead of ""). Includers, if
 /// non-empty, indicates where the \#including file(s) are, in case a relative
 /// search is needed. Microsoft mode will pass all \#including files.
 const FileEntry *HeaderSearch::LookupFile(
     StringRef Filename, SourceLocation IncludeLoc, bool isAngled,
if (llvm::sys::path::is_absolute(Filename)) {
     if (RelativePath != NULL) {
       RelativePath->clear();
       RelativePath->append(Filename.begin(), Filename.end());
     }
     // Otherwise, just return the file.
     return FileMgr.getFile(Filename, /*openFile=*/true);
   }
 
+  // This is the header that MSVC's header search would have found.
+  const FileEntry *MSFE = 0;
+
   // Unless disabled, check to see if the file is in the #includer's
   // directory.  This cannot be based on CurDir, because each includer could be
   // a #include of a subdirectory (#include "foo/bar.h") and a subsequent
   // include of "baz.h" should resolve to "whatever/foo/baz.h".
   // This search is not done for <> headers.
   if (!Includers.empty() && !isAngled && !NoCurDirSearch) {
     SmallString<1024> TmpDir;
-    for (ArrayRef<const FileEntry *>::iterator I(Includers.begin()),
-         E(Includers.end());
+    for (ArrayRef<const FileEntry *>::iterator I = Includers.begin(),
+                                               E = Includers.end();
          I != E; ++I) {
       const FileEntry *Includer = *I;
       // Concatenate the requested file onto the directory.
       // FIXME: Portability.  Filename concatenation should be in sys::Path.
       TmpDir = Includer->getDir()->getName();
       TmpDir.push_back('/');
       TmpDir.append(Filename.begin(), Filename.end());
       if (const FileEntry *FE =
for (ArrayRef<const FileEntry *>::iterator I = Includers.begin(),
           StringRef SearchPathRef(Includer->getDir()->getName());
           SearchPath->clear();
           SearchPath->append(SearchPathRef.begin(), SearchPathRef.end());
         }
         if (RelativePath != NULL) {
           RelativePath->clear();
           RelativePath->append(Filename.begin(), Filename.end());
         }
-        if (I != Includers.begin())
-          Diags.Report(IncludeLoc, diag::ext_pp_include_search_ms)
-              << FE->getName();
+        if (I == Includers.begin())
+          return FE;
+
+        // Otherwise, we found the path via MSVC header search rules.  If
+        // -Wmsvc-include is enabled, we have to keep searching to see if we
+        // would've found this header in -I or -isystem directories.
+        if (Diags.getDiagnosticLevel(diag::ext_pp_include_search_ms,
+                                     IncludeLoc) ==
+            DiagnosticsEngine::Ignored) {
           return FE;
+        } else {
+          MSFE = FE;
+          break;
+        }
       }
     }
   }
 
   CurDir = 0;
 
   // If this is a system #include, ignore the user #include locs.
   unsigned i = isAngled ? AngledDirIdx : 0;
for (; i != SearchDirs.size(); ++i) {
     if (CurDir->isIndexHeaderMap()) {
       size_t SlashPos = Filename.find('/');
       if (SlashPos != StringRef::npos) {
         HFI.IndexHeaderMapHeader = 1;
         HFI.Framework = getUniqueFrameworkName(StringRef(Filename.begin(), 
                                                          SlashPos));
       }
     }
-    
+    if (checkMSVCHeaderSearch(Diags, MSFE, FE, IncludeLoc))
+      return MSFE;
+
     // Remember this location for the next lookup we do.
     CacheLookup.second = i;
     return FE;
   }
 
   // If we are including a file with a quoted include "foo.h" from inside
   // a header in a framework that is currently being built, and we couldn't
   // resolve "foo.h" any other way, change the include to <Foo/foo.h>, where
   // "Foo" is the name of the framework in which the including header was found.
   if (!Includers.empty() && !isAngled &&
       Filename.find('/') == StringRef::npos) {
     HeaderFileInfo &IncludingHFI = getFileInfo(Includers.front());
     if (IncludingHFI.IndexHeaderMapHeader) {
       SmallString<128> ScratchFilename;
       ScratchFilename += IncludingHFI.Framework;
       ScratchFilename += '/';
       ScratchFilename += Filename;
 
-      const FileEntry *Result = LookupFile(
+      const FileEntry *FE = LookupFile(
           ScratchFilename, IncludeLoc, /*isAngled=*/true, FromDir, CurDir,
           Includers.front(), SearchPath, RelativePath, SuggestedModule);
+
+      if (checkMSVCHeaderSearch(Diags, MSFE, FE, IncludeLoc))
+        return MSFE;
+
       std::pair<unsigned, unsigned> &CacheLookup 
         = LookupFileCache.GetOrCreateValue(Filename).getValue();
       CacheLookup.second
         = LookupFileCache.GetOrCreateValue(ScratchFilename).getValue().second;
-      return Result;
+      return FE;
     }
   }
 
+  if (checkMSVCHeaderSearch(Diags, MSFE, 0, IncludeLoc))
+    return MSFE;
+
   // Otherwise, didn't find it. Remember we didn't find this.
   CacheLookup.second = SearchDirs.size();
   return 0;
 }
 
 /// LookupSubframeworkHeader - Look up a subframework for the specified
 /// \#include file.  For example, if \#include'ing <HIToolbox/HIToolbox.h> from
 /// within ".../Carbon.framework/Headers/Carbon.h", check to see if HIToolbox

cfe/trunk/test/Preprocessor/Inputs/microsoft-header-search/a/b/include3.h

 #pragma once
 
-#include "findme.h"
+#include "findme.h"
+
+#include "falsepos.h"

cfe/trunk/test/Preprocessor/microsoft-header-search.c

 // RUN: %clang_cc1 -I%S/Inputs/microsoft-header-search %s -fms-compatibility -verify
 
 // expected-warning@Inputs/microsoft-header-search/a/findme.h:3 {{findme.h successfully included using MS search rules}}
 // expected-warning@Inputs/microsoft-header-search/a/b/include3.h:3 {{#include resolved using non-portable MSVC search rules as}}
 
+// expected-warning@Inputs/microsoft-header-search/falsepos.h:3 {{successfully resolved the falsepos.h header}}
+
 #include "Inputs/microsoft-header-search/include1.h"