[PATCH] D51333: Diagnose likely typos in include statements

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: rsmith.
aaron.ballman added inline comments.


================
Comment at: lib/Lex/PPDirectives.cpp:1876
+                                              "\"" + Filename.str() + "\"")
+              << isFileNotFoundLikelyTypo;
         }
----------------
I'd pass `false` directly here...


================
Comment at: lib/Lex/PPDirectives.cpp:1882
+      if (!File) {
+        // Assuming filename logically starts and end with alphnumeric
+        // character
----------------
... and lower `isFileNotFoundLikelyTypo` to here and rename it `IsFileNotFoundLikelyTypo` to meet our usual naming conventions...


================
Comment at: lib/Lex/PPDirectives.cpp:1887-1888
+          isFileNotFoundLikelyTypo = true;
+          Diag(FilenameTok, diag::err_pp_file_not_found)
+              << Filename << isFileNotFoundLikelyTypo;
+        }
----------------
...then remove this call to `Diag()`...


================
Comment at: lib/Lex/PPDirectives.cpp:1890-1891
+        }
+        Diag(FilenameTok, diag::err_pp_file_not_found)
+            << Filename << FilenameRange << isFileNotFoundLikelyTypo;
+      }
----------------
As it stands, this is going to diagnose the same issue twice.


================
Comment at: test/Preprocessor/include-likely-typo.c:1-2
+// RUN: not %clang_cc1 -verify -frewrite-includes
+#include "<hello.h>" @expected-error {{'<hello.h>' file not found, possibly due to leading or trailing non-alphanumeric characters in the file name}}
----------------
I don't think this is the correct formulation for the test, which explains why it's passing for you. I think the test should be:
```
// RUN: %clang_cc1 %s -verify

#include "<hello.h>" // expected-error {{'<hello.h>' file not found, possibly due to leading or trailing non-alphanumeric characters in the file name}}
```


https://reviews.llvm.org/D51333