[PATCH] D56631: [MSVC Compat] Fix typo correction for inclusion directives.

2019-01-14 Thread Christy Lee via Phabricator via cfe-commits
christylee accepted this revision.
christylee added a comment.
This revision is now accepted and ready to land.

Thanks for catching that!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56631/new/

https://reviews.llvm.org/D56631



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52280: Don't trim non-alphanumeric characters in 'file not found' errors for include directives.

2018-09-20 Thread Christy Lee via Phabricator via cfe-commits
christylee accepted this revision.
christylee added a comment.
This revision is now accepted and ready to land.

Thanks for the fix!


Repository:
  rC Clang

https://reviews.llvm.org/D52280



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D51333: Diagnose likely typos in include statements

2018-09-13 Thread Christy Lee via Phabricator via cfe-commits
christylee updated this revision to Diff 165361.
christylee added a comment.

Added tests.


https://reviews.llvm.org/D51333

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Lex/PPDirectives.cpp
  test/Preprocessor/empty_file_to_include.h
  test/Preprocessor/include-likely-typo.c


Index: test/Preprocessor/include-likely-typo.c
===
--- /dev/null
+++ test/Preprocessor/include-likely-typo.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "" // expected-error 
{{'' file not found, did you mean 
'empty_file_to_include.h'?}}
Index: test/Preprocessor/empty_file_to_include.h
===
--- /dev/null
+++ test/Preprocessor/empty_file_to_include.h
@@ -0,0 +1,7 @@
+
+#ifndef EMPTY_FILE_TO_INCLUDE_H
+#define EMPTY_FILE_TO_INCLUDE_H
+
+// empty file
+
+#endif
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1879,12 +1879,40 @@
 Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
 if (File) {
   SourceRange Range(FilenameTok.getLocation(), CharEnd);
-  Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) <<
+  Diag(FilenameTok, 
diag::err_pp_file_not_found_angled_include_not_fatal) <<
 Filename <<
 FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\"");
 }
   }
 
+  // Check for likely typos due to leading or trailing non-isAlphanumeric
+  // characters
+  if (!File) {
+StringRef OriginalFilename = Filename;
+while (!isAlphanumeric(Filename.front())) {
+  Filename = Filename.drop_front();
+}
+while (!isAlphanumeric(Filename.back())) {
+  Filename = Filename.drop_back();
+}
+
+File = LookupFile(
+FilenameLoc,
+LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
+LookupFrom, LookupFromFile, CurDir,
+Callbacks ? &SearchPath : nullptr,
+Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
+if (File) {
+  SourceRange Range(FilenameTok.getLocation(), CharEnd);
+  auto Hint = isAngled ? FixItHint::CreateReplacement(
+ Range, "<" + Filename.str() + ">")
+   : FixItHint::CreateReplacement(
+ Range, "\"" + Filename.str() + "\"");
+  Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal)
+  << OriginalFilename << Filename << Hint;
+}
+  }
+
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
 Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -422,8 +422,10 @@
   "#pragma hdrstop filename not supported, "
   "/Fp can be used to specify precompiled header filename">,
   InGroup;
-def err_pp_file_not_found_not_fatal : Error<
+def err_pp_file_not_found_angled_include_not_fatal : Error<
   "'%0' file not found with  include; use \"quotes\" instead">;
+def err_pp_file_not_found_typo_not_fatal
+: Error<"'%0' file not found, did you mean '%1'?">;
 def err_pp_error_opening_file : Error<
   "error opening file '%0': %1">, DefaultFatal;
 def err_pp_empty_filename : Error<"empty filename">;


Index: test/Preprocessor/include-likely-typo.c
===
--- /dev/null
+++ test/Preprocessor/include-likely-typo.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "" // expected-error {{'' file not found, did you mean 'empty_file_to_include.h'?}}
Index: test/Preprocessor/empty_file_to_include.h
===
--- /dev/null
+++ test/Preprocessor/empty_file_to_include.h
@@ -0,0 +1,7 @@
+
+#ifndef EMPTY_FILE_TO_INCLUDE_H
+#define EMPTY_FILE_TO_INCLUDE_H
+
+// empty file
+
+#endif
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1879,12 +1879,40 @@
 Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
 if (File) {
   SourceRange Range(FilenameTok.getLocation(), CharEnd);
-  Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) <<
+  Diag(FilenameTok, diag::err_pp_file_not_found_angled_include_not_fatal) <<
 Filename <<
 FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\"");
 }
   }
 
+  // Check for likely typos due to leading or trailing non-isAlphanumeric
+  //

[PATCH] D51333: Diagnose likely typos in include statements

2018-09-13 Thread Christy Lee via Phabricator via cfe-commits
christylee added a comment.

@rsmith , thanks for the review, I fixed the variable capitalization.  If you 
could land it for me that'll be awesome!

In https://reviews.llvm.org/D51333#1233042, @sammccall wrote:

> One thing I'd like to do sometime is add code completion of filenames in 
> #include directives.
>  This would mean listing the relevant directories from the include path. (VFS 
> is slow for this today, but I have a patch out for it).
>
> Maybe it would make sense to do that here and use edit-distance to pick the 
> suggestion, like typo correction elsewhere?
>  Could be a way to extend this patch's behavior in the future.


I also think it's a good idea.  I'm happy to take a crack at it!


https://reviews.llvm.org/D51333



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D51333: Diagnose likely typos in include statements

2018-09-13 Thread Christy Lee via Phabricator via cfe-commits
christylee updated this revision to Diff 165350.

https://reviews.llvm.org/D51333

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Lex/PPDirectives.cpp


Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1879,12 +1879,40 @@
 Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
 if (File) {
   SourceRange Range(FilenameTok.getLocation(), CharEnd);
-  Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) <<
+  Diag(FilenameTok, 
diag::err_pp_file_not_found_angled_include_not_fatal) <<
 Filename <<
 FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\"");
 }
   }
 
+  // Check for likely typos due to leading or trailing non-isAlphanumeric
+  // characters
+  if (!File) {
+StringRef OriginalFilename = Filename;
+while (!isAlphanumeric(Filename.front())) {
+  Filename = Filename.drop_front();
+}
+while (!isAlphanumeric(Filename.back())) {
+  Filename = Filename.drop_back();
+}
+
+File = LookupFile(
+FilenameLoc,
+LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
+LookupFrom, LookupFromFile, CurDir,
+Callbacks ? &SearchPath : nullptr,
+Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
+if (File) {
+  SourceRange Range(FilenameTok.getLocation(), CharEnd);
+  auto Hint = isAngled ? FixItHint::CreateReplacement(
+ Range, "<" + Filename.str() + ">")
+   : FixItHint::CreateReplacement(
+ Range, "\"" + Filename.str() + "\"");
+  Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal)
+  << OriginalFilename << Filename << Hint;
+}
+  }
+
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
 Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -422,8 +422,10 @@
   "#pragma hdrstop filename not supported, "
   "/Fp can be used to specify precompiled header filename">,
   InGroup;
-def err_pp_file_not_found_not_fatal : Error<
+def err_pp_file_not_found_angled_include_not_fatal : Error<
   "'%0' file not found with  include; use \"quotes\" instead">;
+def err_pp_file_not_found_typo_not_fatal
+: Error<"'%0' file not found, did you mean '%1'?">;
 def err_pp_error_opening_file : Error<
   "error opening file '%0': %1">, DefaultFatal;
 def err_pp_empty_filename : Error<"empty filename">;


Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1879,12 +1879,40 @@
 Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
 if (File) {
   SourceRange Range(FilenameTok.getLocation(), CharEnd);
-  Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) <<
+  Diag(FilenameTok, diag::err_pp_file_not_found_angled_include_not_fatal) <<
 Filename <<
 FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\"");
 }
   }
 
+  // Check for likely typos due to leading or trailing non-isAlphanumeric
+  // characters
+  if (!File) {
+StringRef OriginalFilename = Filename;
+while (!isAlphanumeric(Filename.front())) {
+  Filename = Filename.drop_front();
+}
+while (!isAlphanumeric(Filename.back())) {
+  Filename = Filename.drop_back();
+}
+
+File = LookupFile(
+FilenameLoc,
+LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
+LookupFrom, LookupFromFile, CurDir,
+Callbacks ? &SearchPath : nullptr,
+Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
+if (File) {
+  SourceRange Range(FilenameTok.getLocation(), CharEnd);
+  auto Hint = isAngled ? FixItHint::CreateReplacement(
+ Range, "<" + Filename.str() + ">")
+   : FixItHint::CreateReplacement(
+ Range, "\"" + Filename.str() + "\"");
+  Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal)
+  << OriginalFilename << Filename << Hint;
+}
+  }
+
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
 Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
Index: include/clang/Basic/DiagnosticLexKinds.td
==

[PATCH] D51333: Diagnose likely typos in include statements

2018-09-13 Thread Christy Lee via Phabricator via cfe-commits
christylee updated this revision to Diff 165349.

https://reviews.llvm.org/D51333

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Lex/PPDirectives.cpp


Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1879,12 +1879,40 @@
 Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
 if (File) {
   SourceRange Range(FilenameTok.getLocation(), CharEnd);
-  Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) <<
+  Diag(FilenameTok, 
diag::err_pp_file_not_found_angled_include_not_fatal) <<
 Filename <<
 FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\"");
 }
   }
 
+  // Check for likely typos due to leading or trailing non-isAlphanumeric
+  // characters
+  if (!File) {
+auto OriginalFilename = Filename;
+while (!isAlphanumeric(Filename.front())) {
+  Filename = Filename.drop_front();
+}
+while (!isAlphanumeric(Filename.back())) {
+  Filename = Filename.drop_back();
+}
+
+File = LookupFile(
+FilenameLoc,
+LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
+LookupFrom, LookupFromFile, CurDir,
+Callbacks ? &SearchPath : nullptr,
+Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
+if (File) {
+  SourceRange Range(FilenameTok.getLocation(), CharEnd);
+  auto Hint = isAngled ? FixItHint::CreateReplacement(
+ Range, "<" + Filename.str() + ">")
+   : FixItHint::CreateReplacement(
+ Range, "\"" + Filename.str() + "\"");
+  Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal)
+  << OriginalFilename << Filename << Hint;
+}
+  }
+
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
 Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -422,8 +422,10 @@
   "#pragma hdrstop filename not supported, "
   "/Fp can be used to specify precompiled header filename">,
   InGroup;
-def err_pp_file_not_found_not_fatal : Error<
+def err_pp_file_not_found_angled_include_not_fatal : Error<
   "'%0' file not found with  include; use \"quotes\" instead">;
+def err_pp_file_not_found_typo_not_fatal
+: Error<"'%0' file not found, did you mean '%1'?">;
 def err_pp_error_opening_file : Error<
   "error opening file '%0': %1">, DefaultFatal;
 def err_pp_empty_filename : Error<"empty filename">;


Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1879,12 +1879,40 @@
 Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
 if (File) {
   SourceRange Range(FilenameTok.getLocation(), CharEnd);
-  Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) <<
+  Diag(FilenameTok, diag::err_pp_file_not_found_angled_include_not_fatal) <<
 Filename <<
 FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\"");
 }
   }
 
+  // Check for likely typos due to leading or trailing non-isAlphanumeric
+  // characters
+  if (!File) {
+auto OriginalFilename = Filename;
+while (!isAlphanumeric(Filename.front())) {
+  Filename = Filename.drop_front();
+}
+while (!isAlphanumeric(Filename.back())) {
+  Filename = Filename.drop_back();
+}
+
+File = LookupFile(
+FilenameLoc,
+LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
+LookupFrom, LookupFromFile, CurDir,
+Callbacks ? &SearchPath : nullptr,
+Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
+if (File) {
+  SourceRange Range(FilenameTok.getLocation(), CharEnd);
+  auto Hint = isAngled ? FixItHint::CreateReplacement(
+ Range, "<" + Filename.str() + ">")
+   : FixItHint::CreateReplacement(
+ Range, "\"" + Filename.str() + "\"");
+  Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal)
+  << OriginalFilename << Filename << Hint;
+}
+  }
+
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
 Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
Index: include/clang/Basic/DiagnosticLexKinds.td

[PATCH] D51333: Diagnose likely typos in include statements

2018-09-13 Thread Christy Lee via Phabricator via cfe-commits
christylee updated this revision to Diff 165311.
christylee added a comment.

Addressed @rsmith 's comments.


https://reviews.llvm.org/D51333

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Lex/PPDirectives.cpp


Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1879,12 +1879,40 @@
 Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
 if (File) {
   SourceRange Range(FilenameTok.getLocation(), CharEnd);
-  Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) <<
+  Diag(FilenameTok, 
diag::err_pp_file_not_found_angled_include_not_fatal) <<
 Filename <<
 FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\"");
 }
   }
 
+  // Check for likely typos due to leading or trailing non-isAlphanumeric
+  // characters
+  if (!File) {
+auto originalFilename = Filename;
+while (!isAlphanumeric(Filename.front())) {
+  Filename = Filename.drop_front();
+}
+while (!isAlphanumeric(Filename.back())) {
+  Filename = Filename.drop_back();
+}
+
+File = LookupFile(
+FilenameLoc,
+LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
+LookupFrom, LookupFromFile, CurDir,
+Callbacks ? &SearchPath : nullptr,
+Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
+if (File) {
+  SourceRange Range(FilenameTok.getLocation(), CharEnd);
+  auto hint = isAngled ? FixItHint::CreateReplacement(
+ Range, "<" + Filename.str() + ">")
+   : FixItHint::CreateReplacement(
+ Range, "\"" + Filename.str() + "\"");
+  Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal)
+  << originalFilename << Filename << hint;
+}
+  }
+
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
 Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -422,8 +422,10 @@
   "#pragma hdrstop filename not supported, "
   "/Fp can be used to specify precompiled header filename">,
   InGroup;
-def err_pp_file_not_found_not_fatal : Error<
+def err_pp_file_not_found_angled_include_not_fatal : Error<
   "'%0' file not found with  include; use \"quotes\" instead">;
+def err_pp_file_not_found_typo_not_fatal
+: Error<"'%0' file not found, did you mean '%1'?">;
 def err_pp_error_opening_file : Error<
   "error opening file '%0': %1">, DefaultFatal;
 def err_pp_empty_filename : Error<"empty filename">;


Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1879,12 +1879,40 @@
 Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
 if (File) {
   SourceRange Range(FilenameTok.getLocation(), CharEnd);
-  Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) <<
+  Diag(FilenameTok, diag::err_pp_file_not_found_angled_include_not_fatal) <<
 Filename <<
 FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\"");
 }
   }
 
+  // Check for likely typos due to leading or trailing non-isAlphanumeric
+  // characters
+  if (!File) {
+auto originalFilename = Filename;
+while (!isAlphanumeric(Filename.front())) {
+  Filename = Filename.drop_front();
+}
+while (!isAlphanumeric(Filename.back())) {
+  Filename = Filename.drop_back();
+}
+
+File = LookupFile(
+FilenameLoc,
+LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
+LookupFrom, LookupFromFile, CurDir,
+Callbacks ? &SearchPath : nullptr,
+Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
+if (File) {
+  SourceRange Range(FilenameTok.getLocation(), CharEnd);
+  auto hint = isAngled ? FixItHint::CreateReplacement(
+ Range, "<" + Filename.str() + ">")
+   : FixItHint::CreateReplacement(
+ Range, "\"" + Filename.str() + "\"");
+  Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal)
+  << originalFilename << Filename << hint;
+}
+  }
+
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
 Diag(FilenameTok, diag::err_pp_file_not_found) << Filename

[PATCH] D51333: Diagnose likely typos in include statements

2018-09-13 Thread Christy Lee via Phabricator via cfe-commits
christylee added inline comments.



Comment at: lib/Lex/PPDirectives.cpp:1901
+FilenameLoc,
+LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, false,
+LookupFrom, LookupFromFile, CurDir,

rsmith wrote:
> You're passing in `false` for `isAngled` here, and producing a double-quoted 
> replacement below. Is that intentional? I would expect that we would preserve 
> the form of the header-name (quoted or angled) and suggest a replacement with 
> the same form.
My mistake, thanks for catching it!


https://reviews.llvm.org/D51333



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D51333: Diagnose likely typos in include statements

2018-09-12 Thread Christy Lee via Phabricator via cfe-commits
christylee updated this revision to Diff 165162.
christylee edited the summary of this revision.
christylee added a comment.

Emit non-fatal error for typo if file exists.


https://reviews.llvm.org/D51333

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Lex/PPDirectives.cpp


Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1879,12 +1879,38 @@
 Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
 if (File) {
   SourceRange Range(FilenameTok.getLocation(), CharEnd);
-  Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) <<
+  Diag(FilenameTok, 
diag::err_pp_file_not_found_angled_include_not_fatal) <<
 Filename <<
 FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\"");
 }
   }
 
+  // Check for likely typos due to leading or trailing non-isAlphanumeric
+  // characters
+  if (!File) {
+auto originalFilename = Filename;
+while (!isAlphanumeric(Filename.front())) {
+  Filename = Filename.drop_front();
+}
+while (!isAlphanumeric(Filename.back())) {
+  Filename = Filename.drop_back();
+}
+
+File = LookupFile(
+FilenameLoc,
+LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, false,
+LookupFrom, LookupFromFile, CurDir,
+Callbacks ? &SearchPath : nullptr,
+Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
+if (File) {
+  SourceRange Range(FilenameTok.getLocation(), CharEnd);
+  Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal)
+  << originalFilename
+  << FixItHint::CreateReplacement(Range,
+  "\"" + Filename.str() + "\"");
+}
+  }
+
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
 Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -422,8 +422,11 @@
   "#pragma hdrstop filename not supported, "
   "/Fp can be used to specify precompiled header filename">,
   InGroup;
-def err_pp_file_not_found_not_fatal : Error<
+def err_pp_file_not_found_angled_include_not_fatal : Error<
   "'%0' file not found with  include; use \"quotes\" instead">;
+def err_pp_file_not_found_typo_not_fatal
+: Error<"'%0' file not found due to leading or trailing non-alphanumeric "
+"characters">;
 def err_pp_error_opening_file : Error<
   "error opening file '%0': %1">, DefaultFatal;
 def err_pp_empty_filename : Error<"empty filename">;


Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1879,12 +1879,38 @@
 Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
 if (File) {
   SourceRange Range(FilenameTok.getLocation(), CharEnd);
-  Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) <<
+  Diag(FilenameTok, diag::err_pp_file_not_found_angled_include_not_fatal) <<
 Filename <<
 FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\"");
 }
   }
 
+  // Check for likely typos due to leading or trailing non-isAlphanumeric
+  // characters
+  if (!File) {
+auto originalFilename = Filename;
+while (!isAlphanumeric(Filename.front())) {
+  Filename = Filename.drop_front();
+}
+while (!isAlphanumeric(Filename.back())) {
+  Filename = Filename.drop_back();
+}
+
+File = LookupFile(
+FilenameLoc,
+LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, false,
+LookupFrom, LookupFromFile, CurDir,
+Callbacks ? &SearchPath : nullptr,
+Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
+if (File) {
+  SourceRange Range(FilenameTok.getLocation(), CharEnd);
+  Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal)
+  << originalFilename
+  << FixItHint::CreateReplacement(Range,
+  "\"" + Filename.str() + "\"");
+}
+  }
+
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
 Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -422,8 +422,

[PATCH] D51333: Diagnose likely typos in include statements

2018-08-31 Thread Christy Lee via Phabricator via cfe-commits
christylee marked 2 inline comments as done.
christylee added a comment.

In https://reviews.llvm.org/D51333#1219938, @rsmith wrote:

> Instead of guessing whether the corrected filename would be valid, why not 
> strip off the leading and trailing non-alphanumeric characters, look up the 
> resulting filename, and find out? If we did that, then not only could we be a 
> lot more confident that we'd found the file that was intended, but we could 
> also recover from the error by including the trimmed filename.


Should we lookup the stripped filename and then error out on a warning along 
the lines of "'' file not found, did you mean 'hello,h'",  or attempt 
to continue to compile using the file we found (i.e. "hello.h").  I'm learning 
towards the former because I'm worried the latter might be "too clever", in 
case the developers actually think  and hello.h are two different 
files.


https://reviews.llvm.org/D51333



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D51333: Diagnose likely typos in include statements

2018-08-29 Thread Christy Lee via Phabricator via cfe-commits
christylee updated this revision to Diff 163132.
christylee edited the summary of this revision.
christylee added a comment.

Merged warning with existing file_not_found_error.


https://reviews.llvm.org/D51333

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Lex/PPDirectives.cpp
  lib/Lex/Pragma.cpp
  test/Preprocessor/include-likely-typo.c


Index: test/Preprocessor/include-likely-typo.c
===
--- /dev/null
+++ test/Preprocessor/include-likely-typo.c
@@ -0,0 +1,2 @@
+// RUN: not %clang_cc1 -verify -frewrite-includes
+#include "" @expected-error {{'' file not found, possibly 
due to leading or trailing non-alphanumeric characters in the file name}}
Index: lib/Lex/Pragma.cpp
===
--- lib/Lex/Pragma.cpp
+++ lib/Lex/Pragma.cpp
@@ -514,7 +514,7 @@
  nullptr, CurDir, nullptr, nullptr, nullptr, nullptr);
   if (!File) {
 if (!SuppressIncludeNotFoundError)
-  Diag(FilenameTok, diag::err_pp_file_not_found) << Filename;
+  Diag(FilenameTok, diag::err_pp_file_not_found) << Filename << 0;
 return;
   }
 
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1859,6 +1859,7 @@
   // If the file could not be located and it was included via angle
   // brackets, we can attempt a lookup as though it were a quoted path to
   // provide the user with a possible fixit.
+  bool isFileNotFoundLikelyTypo = false;
   if (isAngled) {
 File = LookupFile(
 FilenameLoc,
@@ -1868,16 +1869,27 @@
 Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
 if (File) {
   SourceRange Range(FilenameTok.getLocation(), CharEnd);
-  Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) <<
-Filename <<
-FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\"");
+  Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal)
+  << Filename
+  << FixItHint::CreateReplacement(Range,
+  "\"" + Filename.str() + "\"")
+  << isFileNotFoundLikelyTypo;
 }
   }
 
   // If the file is still not found, just go with the vanilla diagnostic
-  if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
-   << FilenameRange;
+  if (!File) {
+// Assuming filename logically starts and end with alphnumeric
+// character
+if (!isAlphanumeric(Filename.front()) ||
+!isAlphanumeric(Filename.back())) {
+  isFileNotFoundLikelyTypo = true;
+  Diag(FilenameTok, diag::err_pp_file_not_found)
+  << Filename << isFileNotFoundLikelyTypo;
+}
+Diag(FilenameTok, diag::err_pp_file_not_found)
+<< Filename << FilenameRange << isFileNotFoundLikelyTypo;
+  }
 }
   }
 
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -403,7 +403,9 @@
 def err_pp_invalid_directive : Error<"invalid preprocessing directive">;
 def err_pp_directive_required : Error<
   "%0 must be used within a preprocessing directive">;
-def err_pp_file_not_found : Error<"'%0' file not found">, DefaultFatal;
+def err_pp_file_not_found : Error<
+  "'%0' file not found%select{|, possibly due to leading or trailing "
+  "non-alphanumeric characters in the file name}1">, DefaultFatal;
 def err_pp_through_header_not_found : Error<
   "'%0' required for precompiled header not found">, DefaultFatal;
 def err_pp_through_header_not_seen : Error<


Index: test/Preprocessor/include-likely-typo.c
===
--- /dev/null
+++ test/Preprocessor/include-likely-typo.c
@@ -0,0 +1,2 @@
+// RUN: not %clang_cc1 -verify -frewrite-includes
+#include "" @expected-error {{'' file not found, possibly due to leading or trailing non-alphanumeric characters in the file name}}
Index: lib/Lex/Pragma.cpp
===
--- lib/Lex/Pragma.cpp
+++ lib/Lex/Pragma.cpp
@@ -514,7 +514,7 @@
  nullptr, CurDir, nullptr, nullptr, nullptr, nullptr);
   if (!File) {
 if (!SuppressIncludeNotFoundError)
-  Diag(FilenameTok, diag::err_pp_file_not_found) << Filename;
+  Diag(FilenameTok, diag::err_pp_file_not_found) << Filename << 0;
 return;
   }
 
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1859,6 +1859,7 @@
   // If the file could not be located and it was included via angle
   

[PATCH] D51333: Diagnose likely typos in include statements

2018-08-27 Thread Christy Lee via Phabricator via cfe-commits
christylee created this revision.
Herald added a subscriber: cfe-commits.

When someone writes

  #include ""

or

  #include " some_file.h "

the compiler returns "file not fond..." with fonts and quotes that may make it 
hard to see there are excess quotes or surprising bytes in the filename.  This 
feature raises a warning about likely typo and prompts for expected include 
statement format when:
a). the code fails to compile and errors on "file not found", and
b). the first or last character of the filename detected is a non-alphanumeric 
character.

Assuming that files are usually logically named and start and end with an 
alphanumeric character, this feature should help shorten the time for future 
debugging due to simple typos.


Repository:
  rC Clang

https://reviews.llvm.org/D51333

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Lex/PPDirectives.cpp
  test/Frontend/include-likely-typo.c


Index: test/Frontend/include-likely-typo.c
===
--- /dev/null
+++ test/Frontend/include-likely-typo.c
@@ -0,0 +1,4 @@
+// RUN: not %clang_cc1 -verify -frewrite-includes
+#include "" @expected-warning {{likely typo, expected "FILENAME" or 
 but filename is ''}}
+@expected-error {{'' file not found}}
+#include " hello.h " @expected-warning {{likely typo, expected "FILENAME" or 
 but filename is ' hello.h '}}
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1875,9 +1875,16 @@
   }
 
   // If the file is still not found, just go with the vanilla diagnostic
-  if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
-   << FilenameRange;
+  if (!File) {
+// Assuming filename logically starts and end with alphnumeric
+// character
+if (!isAlphanumeric(Filename.front()) ||
+!isAlphanumeric(Filename.back())) {
+  Diag(FilenameTok, diag::warn_pp_filename_likely_typo) << Filename;
+}
+Diag(FilenameTok, diag::err_pp_file_not_found)
+<< Filename << FilenameRange;
+  }
 }
   }
 
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -471,6 +471,10 @@
 def err_pp_identifier_arg_not_identifier : Error<
   "cannot convert %0 token to an identifier">;
 
+def warn_pp_filename_likely_typo :
+  ExtWarn<"likely typo, expected \"FILENAME\" or  "
+  "but filename is '%0'">, InGroup;
+
 def warn_pragma_include_alias_mismatch_angle :
ExtWarn<"angle-bracketed include <%0> cannot be aliased to double-quoted "
"include \"%1\"">, InGroup;


Index: test/Frontend/include-likely-typo.c
===
--- /dev/null
+++ test/Frontend/include-likely-typo.c
@@ -0,0 +1,4 @@
+// RUN: not %clang_cc1 -verify -frewrite-includes
+#include "" @expected-warning {{likely typo, expected "FILENAME" or  but filename is ''}}
+@expected-error {{'' file not found}}
+#include " hello.h " @expected-warning {{likely typo, expected "FILENAME" or  but filename is ' hello.h '}}
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1875,9 +1875,16 @@
   }
 
   // If the file is still not found, just go with the vanilla diagnostic
-  if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
-   << FilenameRange;
+  if (!File) {
+// Assuming filename logically starts and end with alphnumeric
+// character
+if (!isAlphanumeric(Filename.front()) ||
+!isAlphanumeric(Filename.back())) {
+  Diag(FilenameTok, diag::warn_pp_filename_likely_typo) << Filename;
+}
+Diag(FilenameTok, diag::err_pp_file_not_found)
+<< Filename << FilenameRange;
+  }
 }
   }
 
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -471,6 +471,10 @@
 def err_pp_identifier_arg_not_identifier : Error<
   "cannot convert %0 token to an identifier">;
 
+def warn_pp_filename_likely_typo :
+  ExtWarn<"likely typo, expected \"FILENAME\" or  "
+  "but filename is '%0'">, InGroup;
+
 def warn_pragma_include_alias_mismatch_angle :
ExtWarn<"angle-bracketed include <%0> cannot be aliased to double-quoted "
"include \"%1\"">, InGroup;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits