On 22 May 2017 at 14:42, Reid Kleckner via cfe-commits < cfe-commits@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@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
