Fix bugs when an included file name is typo corrected.

[thakis]

Summary

D52774 fixed a bug with typo correction of includes, but didn't add
a test.

D65907 then broke recovery of typo correction of includes again,
because it extracted the code that writes to Filename to a separate
function that took the parameter not by reference.

Fix that, and also don't repeat the slash normalization computation
and fix both lookup and regular file name after recovery.

[hans]

lgtm

[abhinavgaba]

clang/test/Lexer/case-insensitive-include-win.c

8

The test is currently failing if path of an existing directory in a network mounted drive, using forward slashes, is in CPATH. (e.g. export CPATH=//abc.def.com/dir1).

The error in that case is:

fatal error: cannot open file
      '//abc.def.com/dir1\?\<path_to_clang_dir>\test\Lexer\Output\case-insensitive-include-win.c.tmp.dir\FOO.h':
      The specified path is invalid.

[abhinavgaba]

clang/test/Lexer/case-insensitive-include-win.c

8

Did you get a chance to look at this?

clang/include/clang/Lex/Preprocessor.h

struct ImportAction {
     ImportAction(ActionKind AK, Module *Mod = nullptr)
         : Kind(AK), ModuleForHeader(Mod) {
       assert((AK == None || Mod || AK == Failure) &&
              "no module for module action");
     }
   };
 
   Optional<FileEntryRef> LookupHeaderIncludeOrImport(
-      const DirectoryLookup *&CurDir, StringRef Filename,
+      const DirectoryLookup *&CurDir, StringRef &Filename,
       SourceLocation FilenameLoc, CharSourceRange FilenameRange,
       const Token &FilenameTok, bool &IsFrameworkFound, bool IsImportDecl,
       bool &IsMapped, const DirectoryLookup *LookupFrom,
-      const FileEntry *LookupFromFile, StringRef LookupFilename,
+      const FileEntry *LookupFromFile, StringRef &LookupFilename,
       SmallVectorImpl<char> &RelativePath, SmallVectorImpl<char> &SearchPath,
       ModuleMap::KnownHeader &SuggestedModule, bool isAngled);
 
   // File inclusion.
   void HandleIncludeDirective(SourceLocation HashLoc, Token &Tok,
                               const DirectoryLookup *LookupFrom = nullptr,
                               const FileEntry *LookupFromFile = nullptr);
   ImportAction

clang/lib/Lex/PPDirectives.cpp

case ImportAction::Failure:
     TheModuleLoader.HadFatalFailure = true;
     IncludeTok.setKind(tok::eof);
     CurLexer->cutOffLexing();
     return;
   }
 }
 
 Optional<FileEntryRef> Preprocessor::LookupHeaderIncludeOrImport(
-    const DirectoryLookup *&CurDir, StringRef Filename,
+    const DirectoryLookup *&CurDir, StringRef& Filename,
     SourceLocation FilenameLoc, CharSourceRange FilenameRange,
     const Token &FilenameTok, bool &IsFrameworkFound, bool IsImportDecl,
     bool &IsMapped, const DirectoryLookup *LookupFrom,
-    const FileEntry *LookupFromFile, StringRef LookupFilename,
+    const FileEntry *LookupFromFile, StringRef& LookupFilename,
     SmallVectorImpl<char> &RelativePath, SmallVectorImpl<char> &SearchPath,
     ModuleMap::KnownHeader &SuggestedModule, bool isAngled) {
   Optional<FileEntryRef> File = LookupFile(
       FilenameLoc, LookupFilename,
       isAngled, LookupFrom, LookupFromFile, CurDir,
       Callbacks ? &SearchPath : nullptr, Callbacks ? &RelativePath : nullptr,
       &SuggestedModule, &IsMapped, &IsFrameworkFound);
   if (File)
if (LangOpts.SpellChecking) {
     auto CorrectTypoFilename = [](llvm::StringRef Filename) {
       Filename = Filename.drop_until(isAlphanumeric);
       while (!Filename.empty() && !isAlphanumeric(Filename.back())) {
         Filename = Filename.drop_back();
       }
       return Filename;
     };
     StringRef TypoCorrectionName = CorrectTypoFilename(Filename);
-
-#ifndef _WIN32
-    // Normalize slashes when compiling with -fms-extensions on non-Windows.
-    // This is unnecessary on Windows since the filesystem there handles
-    // backslashes.
-    SmallString<128> NormalizedTypoCorrectionPath;
-    if (LangOpts.MicrosoftExt) {
-      NormalizedTypoCorrectionPath = TypoCorrectionName;
-      llvm::sys::path::native(NormalizedTypoCorrectionPath);
-      TypoCorrectionName = NormalizedTypoCorrectionPath;
-    }
-#endif
+    StringRef TypoCorrectionLookupName = CorrectTypoFilename(LookupFilename);
 
     Optional<FileEntryRef> File = LookupFile(
-        FilenameLoc, TypoCorrectionName, isAngled, LookupFrom, LookupFromFile,
+        FilenameLoc, TypoCorrectionLookupName, isAngled, LookupFrom, LookupFromFile,
         CurDir, Callbacks ? &SearchPath : nullptr,
         Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped,
         /*IsFrameworkFound=*/nullptr);
     if (File) {
       auto Hint =
           isAngled ? FixItHint::CreateReplacement(
                          FilenameRange, "<" + TypoCorrectionName.str() + ">")
                    : FixItHint::CreateReplacement(
                          FilenameRange, "\"" + TypoCorrectionName.str() + "\"");
       Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal)
           << OriginalFilename << TypoCorrectionName << Hint;
       // We found the file, so set the Filename to the name after typo
       // correction.
       Filename = TypoCorrectionName;
+      LookupFilename = TypoCorrectionLookupName;
       return File;
     }
   }
 
   // If the file is still not found, just go with the vanilla diagnostic
   assert(!File.hasValue() && "expected missing file");
   Diag(FilenameTok, diag::err_pp_file_not_found)
       << OriginalFilename << FilenameRange;
if (Action == Enter && File && PreambleConditionalStack.isRecording() &&
     Diag(FilenameTok.getLocation(),
          diag::err_pp_including_mainfile_in_preamble);
     return {ImportAction::None};
   }
 
   if (Callbacks && !IsImportDecl) {
     // Notify the callback object that we've seen an inclusion directive.
     // FIXME: Use a different callback for a pp-import?
-    // FIXME: Passes wrong filename if LookupHeaderIncludeOrImport() did
-    // typo correction.
     Callbacks->InclusionDirective(
         HashLoc, IncludeTok, LookupFilename, isAngled, FilenameRange,
         File ? &File->getFileEntry() : nullptr, SearchPath, RelativePath,
         Action == Import ? SuggestedModule.getModule() : nullptr,
         FileCharacter);
     if (Action == Skip && File)
       Callbacks->FileSkipped(*File, FilenameTok, FileCharacter);
   }
#endif
   }
 
   // Issue a diagnostic if the name of the file on disk has a different case
   // than the one we're about to open.
   const bool CheckIncludePathPortability =
       !IsMapped && !File->getFileEntry().tryGetRealPathName().empty();
 
   if (CheckIncludePathPortability) {
-    // FIXME: Looks at the wrong filename if we did typo correction.
     StringRef Name = LookupFilename;
     StringRef NameWithoriginalSlashes = Filename;
 #if defined(_WIN32)
     // Skip UNC prefix if present. (tryGetRealPathName() always
     // returns a path with the prefix skipped.)
     bool NameWasUNC = Name.consume_front("\\\\?\\");
     NameWithoriginalSlashes.consume_front("\\\\?\\");
 #endif

clang/test/Lexer/case-insensitive-include-pr31836.sh

 // REQUIRES: case-insensitive-filesystem
 
 // RUN: mkdir -p %t
 // RUN: touch %t/case-insensitive-include-pr31836.h
-// RUN: echo "#include \"%t/Case-Insensitive-Include-Pr31836.h\"" | %clang_cc1 -E - 2>&1 | FileCheck %s
+// RUN: echo "#include \"\\\\\\\\?\\\\%t/Case-Insensitive-Include-Pr31836.h\"" | not %clang_cc1 -E - 2>&1 | FileCheck %s
 
+// CHECK: error: {{.*}}file not found, did you mean
 // CHECK: warning: non-portable path to file
 // CHECK-SAME: /case-insensitive-include-pr31836.h

clang/test/Lexer/case-insensitive-include-win.c

 // Most Microsoft-specific testing should go in case-insensitive-include-ms.c
 // This file should only include code that really needs a Windows host OS to
 // run.
 
 // REQUIRES: system-windows
 // RUN: mkdir -p %t.dir
 // RUN: touch %t.dir/foo.h
-// RUN: not %clang_cl /FI\\?\%t.dir\FOO.h /WX -Xclang -verify -fsyntax-only %s 2>&1 | FileCheck %s
+// RUN: not %clang_cl /FI\\?\%t.dir\FOO.h /WX -fsyntax-only %s 2>&1 | FileCheck %s
 
 // CHECK: non-portable path to file '"\\?\