[PATCH] D51333: Diagnose likely typos in include statements

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 28 12:12:24 PDT 2018


aaron.ballman added inline comments.


================
Comment at: include/clang/Basic/DiagnosticLexKinds.td:476
+  ExtWarn<"likely typo, expected \"FILENAME\" or <FILENAME> "
+  "but filename is '%0'">, InGroup<UnknownPragmas>;
+
----------------
This seems like the wrong warning group for this diagnostic as it doesn't relate to pragmas. However, we may not want this exposed as a separate warning, either.


================
Comment at: lib/Lex/PPDirectives.cpp:1885-1886
+        }
+        Diag(FilenameTok, diag::err_pp_file_not_found)
+            << Filename << FilenameRange;
+      }
----------------
I don't think we want to produce two different diagnostics for the same line of code. What if, instead, we augment the error diagnostic so that it can produce additional information in this case?
```
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;
```
(or something along those lines.)


================
Comment at: test/Frontend/include-likely-typo.c:1-4
+// RUN: not %clang_cc1 -verify -frewrite-includes
+#include "<hello.h>" @expected-warning {{likely typo, expected "FILENAME" or <FILENAME> but filename is '<hello.h>'}}
+ at expected-error {{'<hello.h>' file not found}}
+#include " hello.h " @expected-warning {{likely typo, expected "FILENAME" or <FILENAME> but filename is ' hello.h '}}
----------------
I think we want this test to live in test/Preprocessor instead of test/Frontend.


Repository:
  rC Clang

https://reviews.llvm.org/D51333





More information about the cfe-commits mailing list