[PATCH] Fix issue 5385: fixit for missing built-in includes

Richard Smith richard at metafoo.co.uk
Wed Nov 12 17:40:38 PST 2014


================
Comment at: include/clang/Lex/Lexer.h:444
@@ +443,3 @@
+  /// \param Directive specifies the desired directive.
+  /// \return returns false if the directive is not found  and
+  /// \p FixitLoc is defaulted to the start of the file.
----------------
Remove the "return" here: "\returns false if ..."

================
Comment at: include/clang/Lex/Lexer.h:451-453
@@ +450,5 @@
+
+  /// \brief Returns the location of the next line adjacent to the source
+  /// location
+  /// the onus is on the caller to check that the location is valid
+  static SourceLocation getLineAfterToken(const SourceLocation &Loc,
----------------
Please start comments with a capital letter and end them with a full stop.

================
Comment at: lib/Lex/Lexer.cpp:1039
@@ +1038,3 @@
+      IncludeLexer.LexFromRawLexer(Tok);
+      if (Tok.getRawIdentifier() == Directive && Tok.getLocation().isValid()) {
+        FoundDirective = true;
----------------
I'm not sure it makes sense to check for a specific directive here. If a file has a mix of `#include`, `#include_next`, and `#import`, we want to find the location after all of them. I think you should make this function less general; specialize it to looking for the "right place to put an include", and just look for the location after the last include-like directive.

Generalizing this to any kind of directive actually makes it less useful, since it prevents us from adding smart heuristics (to deal with `#if`s, to try to keep `#include`s in sorted order, and so on) that only make sense for includes.

================
Comment at: lib/Lex/Lexer.cpp:1074-1081
@@ +1073,10 @@
+
+  while (1) {
+    if (C == '\n' || C == '\r')
+      break;
+    if (RawBuffer == BuffEnd)
+      return SourceLocation();
+    C = *(++RawBuffer);
+    OffSet++;
+  }
+  OffSet++;
----------------
This doesn't cover all the cases. If a line ends in a `\` (or `??/`), or within a `/*...*/` comment, we should keep scanning. One way to handle this would be to build a `Lexer` with whitespace retention enabled, and lex tokens until you find a token at the start of a line.

http://reviews.llvm.org/D5770






More information about the cfe-commits mailing list