Fix issue 5385: fixit for missing built-in includes

[citisolo]

Summary

When a built-in function was used without including the header a warning and the note 'please include the header' was generated without any fixit information . I have created a patch that adds the fixit advice.

[rsmith]

Thank you for working on this, and sorry it took so long for you to get review feedback.

Most of my comments relate to coding style; please review our style rules at http://llvm.org/docs/CodingStandards.html. You can address many of these comments automatically by using clang-format.

Please only do the work of figuring out the right location for a new #include in the case where we've already decided to emit a diagnostic with a fixit, since it's not especially cheap. Please also factor this out into a separate function, for clarity and because we have other cases where we want to do the same thing (missing module imports, for instance).

lib/Sema/SemaDecl.cpp

1686

In the Clang coding style, comments should start with a capital letter and end in a full stop, and there should be a space after the leading //.

1687–1689

I don't think this will do the right thing if the use is within a macro; you will most likely need to pass Loc through SourceMgr.getFileLoc first.

1688

The * should be on the right, not on the left, in Clang code.

1692

Please insert a space after each comma here.

1694

Indent by 2 spaces, not 4.

1698

You should also check Tok.isAtStartOfLine() here.

1702

If you hit a token that isn't part of a preprocessor directive, you should stop scanning. (A #include half way through a file is probably a .def file or similar, and not a normal include, and in any case, your search will be more efficient.)

1707

You shouldn't use a presumed location here; the FixitLoc should refer to a position within the original source buffer, not into the presumed line numbers space.

1708

Please stay wrap to an 80 column line limit.

1709–1711

No braces around a single-line if body.

[citisolo]

made all the discussed changes to format as well as the below changes;
Lexer.h/.cpp : Added two new methods , findLastDirective and getLineAfterToken
SemaDecl.cpp: modified the code to use new methods
implicit-builtin-decl.c: minor modification to make test pass

[citisolo]

tidied up some leftover comments

[rsmith]

include/clang/Lex/Lexer.h

444 ↗

(On Diff #15606)

Remove the "return" here: "\returns false if ..."

451–453 ↗

(On Diff #15606)

Please start comments with a capital letter and end them with a full stop.

lib/Lex/Lexer.cpp

1039 ↗

(On Diff #15606)

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 #ifs, to try to keep #includes in sorted order, and so on) that only make sense for includes.

1074–1081 ↗

(On Diff #15606)

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.

lib/Sema/SemaDecl.cpp

     return nullptr;
   }
 
+  //get the file buffer
+  FileID FileID = SourceMgr.getFileID(Loc);
+  llvm::MemoryBuffer* FileBuf = SourceMgr.getBuffer(FileID);
+  SourceLocation LastIncludeLoc = SourceMgr.getLocForStartOfFile(FileID);
+
+  //Find the Location of the last #include directive in the file
+  Lexer IncludeLexer(FileID,FileBuf,SourceMgr,LangOpts);
+  while (1) {
+      Token Tok;
+      IncludeLexer.LexFromRawLexer(Tok);
+      if (Tok.is(tok::eof))
+          break;
+      if (Tok.is(tok::hash)){
+          IncludeLexer.LexFromRawLexer(Tok);
+          if(Tok.getRawIdentifier() == "include" && Tok.getLocation().isValid())
+                LastIncludeLoc = Tok.getLocation();
+      }
+  }
+
+
+  //Get the location of the line after the last #include directive
+  PresumedLoc PLoc = SourceMgr.getPresumedLoc(LastIncludeLoc);
+  SourceLocation FixitLoc = SourceMgr.translateLineCol(FileID,PLoc.getLine()+1,PLoc.getColumn());
+  if (FixitLoc.isInvalid()){
+      FixitLoc = LastIncludeLoc;
+  }
+
+
+
   if (!ForRedeclaration && Context.BuiltinInfo.isPredefinedLibFunction(ID)) {
     Diag(Loc, diag::ext_implicit_lib_function_decl)
       << Context.BuiltinInfo.GetName(ID)
       << R;
+    //Build 'insert missing header' string
+    std::string FixitString = "#include <";
+    FixitString.append(Context.BuiltinInfo.getHeaderName(ID));
+    FixitString.append(">");
+    StringRef FixitStringRef(FixitString.c_str());
+
     if (Context.BuiltinInfo.getHeaderName(ID) &&
         !Diags.isIgnored(diag::ext_implicit_lib_function_decl, Loc))
-      Diag(Loc, diag::note_include_header_or_declare)
+      Diag(FixitLoc, diag::note_include_header_or_declare)
           << Context.BuiltinInfo.getHeaderName(ID)
-          << Context.BuiltinInfo.GetName(ID);
+          << Context.BuiltinInfo.GetName(ID)
+          << FixItHint::CreateInsertion(FixitLoc, FixitStringRef);
   }
 
   DeclContext *Parent = Context.getTranslationUnitDecl();

test/Sema/Inputs/dummyheader.h

+//this is a dummy header used by implicit-builtin-include-fixit.c

test/Sema/implicit-builtin-decl.c

 
 void f() {
   int *ptr = malloc(sizeof(int) * 10); // expected-warning{{implicitly declaring library function 'malloc' with type}} \
-  // expected-note{{include the header <stdlib.h> or explicitly provide a declaration for 'malloc'}} \
+  // expected-note@-3{{include the header <stdlib.h> or explicitly provide a declaration for 'malloc'}} \
   // expected-note{{'malloc' is a builtin with type 'void *}}
 }
 

test/Sema/implicit-builtin-include-fixit.c

+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wdocumentation -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+#include "Inputs/dummyheader.h"/*dummy header to test implicit declaration fixit*/
+
+void f() {
+  int *ptr = malloc(sizeof(int) * 10); // expected-warning{{implicitly declaring library function 'malloc' with type}}\
+  // expected-note@-2{{include the header <stdlib.h> or explicitly provide a declaration for 'malloc'}}  
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:1-[[@LINE-4]]:1}:"#include <stdlib.h>"
+}