r303582 - Give files from #line the characteristics of the current file

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Mon May 22 15:17:53 PDT 2017


On 22 May 2017 at 14:42, Reid Kleckner via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: rnk
> Date: Mon May 22 16:42:58 2017
> New Revision: 303582
>
> URL: http://llvm.org/viewvc/llvm-project?rev=303582&view=rev
> Log:
> Give files from #line the characteristics of the current file
>
> This allows #line directives to appear in system headers that have code
> that clang would normally warn on. This is compatible with GCC, which is
> easy to test by running `gcc -E`.
>
> Fixes PR30752
>
> Added:
>     cfe/trunk/test/Frontend/Inputs/SystemHeaderPrefix/line.h
>     cfe/trunk/test/Frontend/Inputs/SystemHeaderPrefix/noline.h
>     cfe/trunk/test/Frontend/system-header-line-directive.c
> Modified:
>     cfe/trunk/include/clang/Basic/SourceManager.h
>     cfe/trunk/include/clang/Basic/SourceManagerInternals.h
>     cfe/trunk/lib/Basic/SourceManager.cpp
>     cfe/trunk/lib/Frontend/FrontendAction.cpp
>     cfe/trunk/lib/Lex/PPDirectives.cpp
>     cfe/trunk/lib/Lex/Pragma.cpp
>
> Modified: cfe/trunk/include/clang/Basic/SourceManager.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Basic/SourceManager.h?rev=303582&r1=303581&r2=303582&view=diff
> ============================================================
> ==================
> --- cfe/trunk/include/clang/Basic/SourceManager.h (original)
> +++ cfe/trunk/include/clang/Basic/SourceManager.h Mon May 22 16:42:58 2017
> @@ -1399,10 +1399,9 @@ public:
>    /// specified by Loc.
>    ///
>    /// If FilenameID is -1, it is considered to be unspecified.
> -  void AddLineNote(SourceLocation Loc, unsigned LineNo, int FilenameID);
>    void AddLineNote(SourceLocation Loc, unsigned LineNo, int FilenameID,
>                     bool IsFileEntry, bool IsFileExit,
> -                   bool IsSystemHeader, bool IsExternCHeader);
> +                   SrcMgr::CharacteristicKind FileKind);
>
>    /// \brief Determine if the source manager has a line table.
>    bool hasLineTable() const { return LineTable != nullptr; }
>
> Modified: cfe/trunk/include/clang/Basic/SourceManagerInternals.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/
> SourceManagerInternals.h?rev=303582&r1=303581&r2=303582&view=diff
> ============================================================
> ==================
> --- cfe/trunk/include/clang/Basic/SourceManagerInternals.h (original)
> +++ cfe/trunk/include/clang/Basic/SourceManagerInternals.h Mon May 22
> 16:42:58 2017
> @@ -102,8 +102,6 @@ public:
>    unsigned getNumFilenames() const { return FilenamesByID.size(); }
>
>    void AddLineNote(FileID FID, unsigned Offset,
> -                   unsigned LineNo, int FilenameID);
> -  void AddLineNote(FileID FID, unsigned Offset,
>                     unsigned LineNo, int FilenameID,
>                     unsigned EntryExit, SrcMgr::CharacteristicKind
> FileKind);
>
>
> Modified: cfe/trunk/lib/Basic/SourceManager.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/
> SourceManager.cpp?rev=303582&r1=303581&r2=303582&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/Basic/SourceManager.cpp (original)
> +++ cfe/trunk/lib/Basic/SourceManager.cpp Mon May 22 16:42:58 2017
> @@ -183,48 +183,22 @@ unsigned LineTableInfo::getLineTableFile
>    return IterBool.first->second;
>  }
>
> -/// AddLineNote - Add a line note to the line table that indicates that
> there
> -/// is a \#line at the specified FID/Offset location which changes the
> presumed
> -/// location to LineNo/FilenameID.
> -void LineTableInfo::AddLineNote(FileID FID, unsigned Offset,
> -                                unsigned LineNo, int FilenameID) {
> -  std::vector<LineEntry> &Entries = LineEntries[FID];
> -
> -  assert((Entries.empty() || Entries.back().FileOffset < Offset) &&
> -         "Adding line entries out of order!");
> -
> -  SrcMgr::CharacteristicKind Kind = SrcMgr::C_User;
> -  unsigned IncludeOffset = 0;
> -
> -  if (!Entries.empty()) {
> -    // If this is a '#line 4' after '#line 42 "foo.h"', make sure to
> remember
> -    // that we are still in "foo.h".
> -    if (FilenameID == -1)
> -      FilenameID = Entries.back().FilenameID;
> -
> -    // If we are after a line marker that switched us to system header
> mode, or
> -    // that set #include information, preserve it.
> -    Kind = Entries.back().FileKind;
> -    IncludeOffset = Entries.back().IncludeOffset;
> -  }
> -
> -  Entries.push_back(LineEntry::get(Offset, LineNo, FilenameID, Kind,
> -                                   IncludeOffset));
> -}
> -
> -/// AddLineNote This is the same as the previous version of AddLineNote,
> but is
> -/// used for GNU line markers.  If EntryExit is 0, then this doesn't
> change the
> -/// presumed \#include stack.  If it is 1, this is a file entry, if it is
> 2 then
> -/// this is a file exit.  FileKind specifies whether this is a system
> header or
> -/// extern C system header.
> -void LineTableInfo::AddLineNote(FileID FID, unsigned Offset,
> -                                unsigned LineNo, int FilenameID,
> -                                unsigned EntryExit,
> +/// Add a line note to the line table that indicates that there is a
> \#line or
> +/// GNU line marker at the specified FID/Offset location which changes the
> +/// presumed location to LineNo/FilenameID. If EntryExit is 0, then this
> doesn't
> +/// change the presumed \#include stack.  If it is 1, this is a file
> entry, if
> +/// it is 2 then this is a file exit. FileKind specifies whether this is a
> +/// system header or extern C system header.
> +void LineTableInfo::AddLineNote(FileID FID, unsigned Offset, unsigned
> LineNo,
> +                                int FilenameID, unsigned EntryExit,
>                                  SrcMgr::CharacteristicKind FileKind) {
> -  assert(FilenameID != -1 && "Unspecified filename should use other
> accessor");
> -
>    std::vector<LineEntry> &Entries = LineEntries[FID];
>
> +  // An unspecified FilenameID means use the last filename if available,
> or the
> +  // main source file otherwise.
> +  if (FilenameID == -1 && !Entries.empty())
> +    FilenameID = Entries.back().FilenameID;
> +
>    assert((Entries.empty() || Entries.back().FileOffset < Offset) &&
>           "Adding line entries out of order!");
>
> @@ -281,47 +255,20 @@ unsigned SourceManager::getLineTableFile
>    return getLineTable().getLineTableFilenameID(Name);
>  }
>
> -
>  /// AddLineNote - Add a line note to the line table for the FileID and
> offset
>  /// specified by Loc.  If FilenameID is -1, it is considered to be
>  /// unspecified.
>  void SourceManager::AddLineNote(SourceLocation Loc, unsigned LineNo,
> -                                int FilenameID) {
> -  std::pair<FileID, unsigned> LocInfo = getDecomposedExpansionLoc(Loc);
> -
> -  bool Invalid = false;
> -  const SLocEntry &Entry = getSLocEntry(LocInfo.first, &Invalid);
> -  if (!Entry.isFile() || Invalid)
> -    return;
> -
> -  const SrcMgr::FileInfo &FileInfo = Entry.getFile();
> -
> -  // Remember that this file has #line directives now if it doesn't
> already.
> -  const_cast<SrcMgr::FileInfo&>(FileInfo).setHasLineDirectives();
> -
> -  getLineTable().AddLineNote(LocInfo.first, LocInfo.second, LineNo,
> FilenameID);
> -}
> -
> -/// AddLineNote - Add a GNU line marker to the line table.
> -void SourceManager::AddLineNote(SourceLocation Loc, unsigned LineNo,
>                                  int FilenameID, bool IsFileEntry,
> -                                bool IsFileExit, bool IsSystemHeader,
> -                                bool IsExternCHeader) {
> -  // If there is no filename and no flags, this is treated just like a
> #line,
> -  // which does not change the flags of the previous line marker.
> -  if (FilenameID == -1) {
> -    assert(!IsFileEntry && !IsFileExit && !IsSystemHeader &&
> !IsExternCHeader &&
> -           "Can't set flags without setting the filename!");
> -    return AddLineNote(Loc, LineNo, FilenameID);
> -  }
> -
> +                                bool IsFileExit,
> +                                SrcMgr::CharacteristicKind FileKind) {
>    std::pair<FileID, unsigned> LocInfo = getDecomposedExpansionLoc(Loc);
>
>    bool Invalid = false;
>    const SLocEntry &Entry = getSLocEntry(LocInfo.first, &Invalid);
>    if (!Entry.isFile() || Invalid)
>      return;
> -
> +
>    const SrcMgr::FileInfo &FileInfo = Entry.getFile();
>
>    // Remember that this file has #line directives now if it doesn't
> already.
> @@ -329,14 +276,6 @@ void SourceManager::AddLineNote(SourceLo
>
>    (void) getLineTable();
>
> -  SrcMgr::CharacteristicKind FileKind;
> -  if (IsExternCHeader)
> -    FileKind = SrcMgr::C_ExternCSystem;
> -  else if (IsSystemHeader)
> -    FileKind = SrcMgr::C_System;
> -  else
> -    FileKind = SrcMgr::C_User;
> -
>    unsigned EntryExit = 0;
>    if (IsFileEntry)
>      EntryExit = 1;
>
> Modified: cfe/trunk/lib/Frontend/FrontendAction.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/
> Frontend/FrontendAction.cpp?rev=303582&r1=303581&r2=303582&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/Frontend/FrontendAction.cpp (original)
> +++ cfe/trunk/lib/Frontend/FrontendAction.cpp Mon May 22 16:42:58 2017
> @@ -252,7 +252,8 @@ static SourceLocation ReadOriginalFileNa
>
>    if (AddLineNote)
>      CI.getSourceManager().AddLineNote(
> -        LineNoLoc, LineNo, SourceMgr.getLineTableFilenameID(InputFile));
> +        LineNoLoc, LineNo, SourceMgr.getLineTableFilenameID(InputFile),
> false,
> +        false, SrcMgr::C_User);
>
>    return T.getLocation();
>  }
>
> Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/
> PPDirectives.cpp?rev=303582&r1=303581&r2=303582&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
> +++ cfe/trunk/lib/Lex/PPDirectives.cpp Mon May 22 16:42:58 2017
> @@ -1171,18 +1171,26 @@ void Preprocessor::HandleLineDirective()
>      CheckEndOfDirective("line", true);
>    }
>
> -  SourceMgr.AddLineNote(DigitTok.getLocation(), LineNo, FilenameID);
> +  // Take the file kind of the file containing the #line directive. #line
> +  // directives are often used for generated sources from the same
> codebase, so
> +  // the new file should generally be classified the same way as the
> current
> +  // file. This is visible in GCC's pre-processed output, which rewrites
> #line
> +  // to GNU line markers.
> +  SrcMgr::CharacteristicKind FileKind =
> +      SourceMgr.getFileCharacteristic(DigitTok.getLocation());
> +
> +  SourceMgr.AddLineNote(DigitTok.getLocation(), LineNo, FilenameID,
> false,
> +                        false, FileKind);
>
>    if (Callbacks)
>      Callbacks->FileChanged(CurPPLexer->getSourceLocation(),
> -                           PPCallbacks::RenameFile,
> -                           SrcMgr::C_User);
> +                           PPCallbacks::RenameFile, FileKind);
>  }
>
>  /// ReadLineMarkerFlags - Parse and validate any flags at the end of a
> GNU line
>  /// marker directive.
>  static bool ReadLineMarkerFlags(bool &IsFileEntry, bool &IsFileExit,
> -                                bool &IsSystemHeader, bool
> &IsExternCHeader,
> +                                SrcMgr::CharacteristicKind &FileKind,
>                                  Preprocessor &PP) {
>    unsigned FlagVal;
>    Token FlagTok;
> @@ -1233,7 +1241,7 @@ static bool ReadLineMarkerFlags(bool &Is
>      return true;
>    }
>
> -  IsSystemHeader = true;
> +  FileKind = SrcMgr::C_System;
>
>    PP.Lex(FlagTok);
>    if (FlagTok.is(tok::eod)) return false;
> @@ -1247,7 +1255,7 @@ static bool ReadLineMarkerFlags(bool &Is
>      return true;
>    }
>
> -  IsExternCHeader = true;
> +  FileKind = SrcMgr::C_ExternCSystem;
>
>    PP.Lex(FlagTok);
>    if (FlagTok.is(tok::eod)) return false;
> @@ -1277,14 +1285,15 @@ void Preprocessor::HandleDigitDirective(
>    Lex(StrTok);
>
>    bool IsFileEntry = false, IsFileExit = false;
> -  bool IsSystemHeader = false, IsExternCHeader = false;
>    int FilenameID = -1;
> +  SrcMgr::CharacteristicKind FileKind = SrcMgr::C_User;
>
>    // If the StrTok is "eod", then it wasn't present.  Otherwise, it must
> be a
>    // string followed by eod.
> -  if (StrTok.is(tok::eod))
> -    ; // ok
> -  else if (StrTok.isNot(tok::string_literal)) {
> +  if (StrTok.is(tok::eod)) {
> +    // Treat this like "#line NN", which doesn't change file
> characteristics.
> +    FileKind = SourceMgr.getFileCharacteristic(DigitTok.getLocation());
>

This change for "# <number>" handling makes sense (and I've checked and it
matches GCC), but it looks like we don't have test coverage for either the
old or new behavior. Can I interest you in adding some? :)

+  } else if (StrTok.isNot(tok::string_literal)) {
>      Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
>      return DiscardUntilEndOfDirective();
>    } else if (StrTok.hasUDSuffix()) {
> @@ -1303,15 +1312,13 @@ void Preprocessor::HandleDigitDirective(
>      FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString());
>
>      // If a filename was present, read any flags that are present.
> -    if (ReadLineMarkerFlags(IsFileEntry, IsFileExit,
> -                            IsSystemHeader, IsExternCHeader, *this))
> +    if (ReadLineMarkerFlags(IsFileEntry, IsFileExit, FileKind, *this))
>        return;
>    }
>
>    // Create a line note with this information.
> -  SourceMgr.AddLineNote(DigitTok.getLocation(), LineNo, FilenameID,
> -                        IsFileEntry, IsFileExit,
> -                        IsSystemHeader, IsExternCHeader);
> +  SourceMgr.AddLineNote(DigitTok.getLocation(), LineNo, FilenameID,
> IsFileEntry,
> +                        IsFileExit, FileKind);
>
>    // If the preprocessor has callbacks installed, notify them of the #line
>    // change.  This is used so that the line marker comes out in -E mode
> for
> @@ -1322,11 +1329,6 @@ void Preprocessor::HandleDigitDirective(
>        Reason = PPCallbacks::EnterFile;
>      else if (IsFileExit)
>        Reason = PPCallbacks::ExitFile;
> -    SrcMgr::CharacteristicKind FileKind = SrcMgr::C_User;
> -    if (IsExternCHeader)
> -      FileKind = SrcMgr::C_ExternCSystem;
> -    else if (IsSystemHeader)
> -      FileKind = SrcMgr::C_System;
>
>      Callbacks->FileChanged(CurPPLexer->getSourceLocation(), Reason,
> FileKind);
>    }
>
> Modified: cfe/trunk/lib/Lex/Pragma.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/
> Pragma.cpp?rev=303582&r1=303581&r2=303582&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/Lex/Pragma.cpp (original)
> +++ cfe/trunk/lib/Lex/Pragma.cpp Mon May 22 16:42:58 2017
> @@ -475,9 +475,9 @@ void Preprocessor::HandlePragmaSystemHea
>    // Emit a line marker.  This will change any source locations from this
> point
>    // forward to realize they are in a system header.
>    // Create a line note with this information.
> -  SourceMgr.AddLineNote(SysHeaderTok.getLocation(), PLoc.getLine()+1,
> +  SourceMgr.AddLineNote(SysHeaderTok.getLocation(), PLoc.getLine() + 1,
>                          FilenameID, /*IsEntry=*/false, /*IsExit=*/false,
> -                        /*IsSystem=*/true, /*IsExternC=*/false);
> +                        SrcMgr::C_System);
>  }
>
>  /// HandlePragmaDependency - Handle \#pragma GCC dependency "foo" blah.
>
> Added: cfe/trunk/test/Frontend/Inputs/SystemHeaderPrefix/line.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/Inputs/
> SystemHeaderPrefix/line.h?rev=303582&view=auto
> ============================================================
> ==================
> --- cfe/trunk/test/Frontend/Inputs/SystemHeaderPrefix/line.h (added)
> +++ cfe/trunk/test/Frontend/Inputs/SystemHeaderPrefix/line.h Mon May 22
> 16:42:58 2017
> @@ -0,0 +1,2 @@
> +#line 1 "foo.h"
> +foo();
>
> Added: cfe/trunk/test/Frontend/Inputs/SystemHeaderPrefix/noline.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/Inputs/
> SystemHeaderPrefix/noline.h?rev=303582&view=auto
> ============================================================
> ==================
> --- cfe/trunk/test/Frontend/Inputs/SystemHeaderPrefix/noline.h (added)
> +++ cfe/trunk/test/Frontend/Inputs/SystemHeaderPrefix/noline.h Mon May 22
> 16:42:58 2017
> @@ -0,0 +1 @@
> +foo();
>
> Added: cfe/trunk/test/Frontend/system-header-line-directive.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> Frontend/system-header-line-directive.c?rev=303582&view=auto
> ============================================================
> ==================
> --- cfe/trunk/test/Frontend/system-header-line-directive.c (added)
> +++ cfe/trunk/test/Frontend/system-header-line-directive.c Mon May 22
> 16:42:58 2017
> @@ -0,0 +1,20 @@
> +// RUN: %clang_cc1 -Wall %s -isystem %S/Inputs/SystemHeaderPrefix -verify
> +// RUN: %clang_cc1 %s -E -o - -isystem %S/Inputs/SystemHeaderPrefix |
> FileCheck %s
> +#include <noline.h>
> +#include <line.h>
> +
> +// This tests that "#line" directives in system headers preserve system
> +// header-ness just like GNU line markers that don't have filenames.
> This was
> +// PR30752.
> +
> +// expected-no-diagnostics
> +
> +// CHECK: # {{[0-9]+}} "{{.*}}system-header-line-directive.c" 2
> +// CHECK: # 1 "{{.*}}noline.h" 1 3
> +// CHECK: foo();
> +// CHECK: # 4 "{{.*}}system-header-line-directive.c" 2
> +// CHECK: # 1 "{{.*}}line.h" 1 3
> +//      The "3" below indicates that "foo.h" is considered a system
> header.
> +// CHECK: # 1 "foo.h" 3
> +// CHECK: foo();
> +// CHECK: # {{[0-9]+}} "{{.*}}system-header-line-directive.c" 2
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170522/ec61a61e/attachment-0001.html>


More information about the cfe-commits mailing list