From fee9e20b903b11c677495825728f017dd705d9fc Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Wed, 28 Jan 2015 20:14:54 +0000 Subject: [PATCH] Fix layering violation: include/clang/Basic/PlistSupport.h should not include files from include/clang/Lex. Clean up module map. llvm-svn: 227361 --- clang/include/clang/Basic/PlistSupport.h | 23 ++++---- clang/include/clang/Basic/SourceManager.h | 8 ++- clang/include/clang/Lex/Lexer.h | 20 +++++++ clang/include/clang/module.modulemap | 68 +++++++++------------- clang/lib/ARCMigrate/PlistReporter.cpp | 14 +++-- clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp | 22 +++---- .../diagnostics/report-issues-within-main-file.cpp | 2 +- clang/test/Analysis/plist-macros.cpp | 6 +- 8 files changed, 88 insertions(+), 75 deletions(-) diff --git a/clang/include/clang/Basic/PlistSupport.h b/clang/include/clang/Basic/PlistSupport.h index 081f22d..84dd291 100644 --- a/clang/include/clang/Basic/PlistSupport.h +++ b/clang/include/clang/Basic/PlistSupport.h @@ -12,7 +12,6 @@ #include "clang/Basic/FileManager.h" #include "clang/Basic/SourceManager.h" -#include "clang/Lex/Lexer.h" #include "llvm/Support/raw_ostream.h" namespace clang { @@ -89,31 +88,29 @@ inline raw_ostream &EmitString(raw_ostream &o, StringRef s) { } inline void EmitLocation(raw_ostream &o, const SourceManager &SM, - const LangOptions &LangOpts, SourceLocation L, - const FIDMap &FM, unsigned indent, - bool extend = false) { - FullSourceLoc Loc(SM.getExpansionLoc(L), const_cast(SM)); + SourceLocation L, const FIDMap &FM, unsigned indent) { + if (L.isInvalid()) return; - // Add in the length of the token, so that we cover multi-char tokens. - unsigned offset = - extend ? Lexer::MeasureTokenLength(Loc, SM, LangOpts) - 1 : 0; + FullSourceLoc Loc(SM.getExpansionLoc(L), const_cast(SM)); Indent(o, indent) << "\n"; Indent(o, indent) << " line"; EmitInteger(o, Loc.getExpansionLineNumber()) << '\n'; Indent(o, indent) << " col"; - EmitInteger(o, Loc.getExpansionColumnNumber() + offset) << '\n'; + EmitInteger(o, Loc.getExpansionColumnNumber()) << '\n'; Indent(o, indent) << " file"; EmitInteger(o, GetFID(FM, SM, Loc)) << '\n'; Indent(o, indent) << "\n"; } inline void EmitRange(raw_ostream &o, const SourceManager &SM, - const LangOptions &LangOpts, CharSourceRange R, - const FIDMap &FM, unsigned indent) { + CharSourceRange R, const FIDMap &FM, unsigned indent) { + if (R.isInvalid()) return; + + assert(R.isCharRange() && "cannot handle a token range"); Indent(o, indent) << "\n"; - EmitLocation(o, SM, LangOpts, R.getBegin(), FM, indent + 1); - EmitLocation(o, SM, LangOpts, R.getEnd(), FM, indent + 1, R.isTokenRange()); + EmitLocation(o, SM, R.getBegin(), FM, indent + 1); + EmitLocation(o, SM, R.getEnd(), FM, indent + 1); Indent(o, indent) << "\n"; } } diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h index 717258d..d17fdd5 100644 --- a/clang/include/clang/Basic/SourceManager.h +++ b/clang/include/clang/Basic/SourceManager.h @@ -1057,10 +1057,16 @@ public: getImmediateExpansionRange(SourceLocation Loc) const; /// \brief Given a SourceLocation object, return the range of - /// tokens covered by the expansion the ultimate file. + /// tokens covered by the expansion in the ultimate file. std::pair getExpansionRange(SourceLocation Loc) const; + /// \brief Given a SourceRange object, return the range of + /// tokens covered by the expansion in the ultimate file. + SourceRange getExpansionRange(SourceRange Range) const { + return SourceRange(getExpansionRange(Range.getBegin()).first, + getExpansionRange(Range.getEnd()).second); + } /// \brief Given a SourceLocation object, return the spelling /// location referenced by the ID. diff --git a/clang/include/clang/Lex/Lexer.h b/clang/include/clang/Lex/Lexer.h index c1f968b..42fe020 100644 --- a/clang/include/clang/Lex/Lexer.h +++ b/clang/include/clang/Lex/Lexer.h @@ -323,6 +323,26 @@ public: const SourceManager &SM, const LangOptions &LangOpts); + /// \brief Given a token range, produce a corresponding CharSourceRange that + /// is not a token range. This allows the source range to be used by + /// components that don't have access to the lexer and thus can't find the + /// end of the range for themselves. + static CharSourceRange getAsCharRange(SourceRange Range, + const SourceManager &SM, + const LangOptions &LangOpts) { + SourceLocation End = getLocForEndOfToken(Range.getEnd(), 0, SM, LangOpts); + return End.isInvalid() ? CharSourceRange() + : CharSourceRange::getCharRange( + Range.getBegin(), End.getLocWithOffset(-1)); + } + static CharSourceRange getAsCharRange(CharSourceRange Range, + const SourceManager &SM, + const LangOptions &LangOpts) { + return Range.isTokenRange() + ? getAsCharRange(Range.getAsRange(), SM, LangOpts) + : Range; + } + /// \brief Returns true if the given MacroID location points at the first /// token of the macro expansion. /// diff --git a/clang/include/clang/module.modulemap b/clang/include/clang/module.modulemap index 5058d15..9e4dac1 100644 --- a/clang/include/clang/module.modulemap +++ b/clang/include/clang/module.modulemap @@ -2,8 +2,7 @@ module Clang_Analysis { requires cplusplus umbrella "Analysis" - // This file is intended for repeated textual inclusion. - exclude header "Analysis/Analyses/ThreadSafetyOps.def" + textual header "Analysis/Analyses/ThreadSafetyOps.def" module * { export * } } @@ -12,10 +11,9 @@ module Clang_AST { requires cplusplus umbrella "AST" - // These files are intended for repeated textual inclusion. - exclude header "AST/BuiltinTypes.def" - exclude header "AST/TypeLocNodes.def" - exclude header "AST/TypeNodes.def" + textual header "AST/BuiltinTypes.def" + textual header "AST/TypeLocNodes.def" + textual header "AST/TypeNodes.def" module * { export * } } @@ -26,33 +24,26 @@ module Clang_Basic { requires cplusplus umbrella "Basic" - // These files are intended for repeated textual inclusion. - exclude header "Basic/BuiltinsAArch64.def" - exclude header "Basic/BuiltinsARM64.def" - exclude header "Basic/BuiltinsARM.def" - exclude header "Basic/Builtins.def" - exclude header "Basic/BuiltinsHexagon.def" - exclude header "Basic/BuiltinsMips.def" - exclude header "Basic/BuiltinsNEON.def" - exclude header "Basic/BuiltinsNVPTX.def" - exclude header "Basic/BuiltinsPPC.def" - exclude header "Basic/BuiltinsR600.def" - exclude header "Basic/BuiltinsX86.def" - exclude header "Basic/BuiltinsXCore.def" - exclude header "Basic/BuiltinsLe64.def" - exclude header "Basic/DiagnosticOptions.def" - exclude header "Basic/LangOptions.def" - exclude header "Basic/OpenCLExtensions.def" - exclude header "Basic/OpenMPKinds.def" - exclude header "Basic/OperatorKinds.def" - exclude header "Basic/Sanitizers.def" - exclude header "Basic/TokenKinds.def" - - // This file includes a header from Lex. - exclude header "Basic/PlistSupport.h" - - // FIXME: This is logically a part of Basic, but has been put in the wrong place. - header "StaticAnalyzer/Core/AnalyzerOptions.h" + textual header "Basic/BuiltinsAArch64.def" + textual header "Basic/BuiltinsARM64.def" + textual header "Basic/BuiltinsARM.def" + textual header "Basic/Builtins.def" + textual header "Basic/BuiltinsHexagon.def" + textual header "Basic/BuiltinsMips.def" + textual header "Basic/BuiltinsNEON.def" + textual header "Basic/BuiltinsNVPTX.def" + textual header "Basic/BuiltinsPPC.def" + textual header "Basic/BuiltinsR600.def" + textual header "Basic/BuiltinsX86.def" + textual header "Basic/BuiltinsXCore.def" + textual header "Basic/BuiltinsLe64.def" + textual header "Basic/DiagnosticOptions.def" + textual header "Basic/LangOptions.def" + textual header "Basic/OpenCLExtensions.def" + textual header "Basic/OpenMPKinds.def" + textual header "Basic/OperatorKinds.def" + textual header "Basic/Sanitizers.def" + textual header "Basic/TokenKinds.def" module * { export * } } @@ -82,8 +73,7 @@ module Clang_Driver { requires cplusplus umbrella "Driver" - // This file is intended for repeated textual inclusion. - exclude header "Driver/Types.def" + textual header "Driver/Types.def" module * { export * } } @@ -95,9 +85,8 @@ module Clang_Frontend { requires cplusplus umbrella "Frontend" - // These files are intended for repeated textual inclusion. - exclude header "Frontend/CodeGenOptions.def" - exclude header "Frontend/LangStandards.def" + textual header "Frontend/CodeGenOptions.def" + textual header "Frontend/LangStandards.def" module * { export * } } @@ -114,8 +103,7 @@ module Clang_StaticAnalyzer { requires cplusplus umbrella "StaticAnalyzer" - // This file is intended for repeated textual inclusion. - exclude header "StaticAnalyzer/Core/Analyses.def" + textual header "StaticAnalyzer/Core/Analyses.def" module * { export * } } diff --git a/clang/lib/ARCMigrate/PlistReporter.cpp b/clang/lib/ARCMigrate/PlistReporter.cpp index 53398b2..9a51690 100644 --- a/clang/lib/ARCMigrate/PlistReporter.cpp +++ b/clang/lib/ARCMigrate/PlistReporter.cpp @@ -100,16 +100,18 @@ void arcmt::writeARCDiagsToPlist(const std::string &outPath, // Output the location of the bug. o << " location\n"; - EmitLocation(o, SM, LangOpts, D.getLocation(), FM, 2); + EmitLocation(o, SM, D.getLocation(), FM, 2); // Output the ranges (if any). - StoredDiagnostic::range_iterator RI = D.range_begin(), RE = D.range_end(); - - if (RI != RE) { + if (!D.getRanges().empty()) { o << " ranges\n"; o << " \n"; - for (; RI != RE; ++RI) - EmitRange(o, SM, LangOpts, *RI, FM, 4); + for (auto &R : D.getRanges()) { + CharSourceRange ExpansionRange(SM.getExpansionRange(R.getAsRange()), + R.isTokenRange()); + EmitRange(o, SM, Lexer::getAsCharRange(ExpansionRange, SM, LangOpts), + FM, 4); + } o << " \n"; } diff --git a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp index a2c66f8..11bd254 100644 --- a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp +++ b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp @@ -106,13 +106,14 @@ static void ReportControlFlow(raw_ostream &o, // by forcing to use only the beginning of the range. This simplifies the layout // logic for clients. Indent(o, indent) << "start\n"; - SourceLocation StartEdge = I->getStart().asRange().getBegin(); - EmitRange(o, SM, LangOpts, CharSourceRange::getTokenRange(StartEdge), FM, + SourceRange StartEdge( + SM.getExpansionLoc(I->getStart().asRange().getBegin())); + EmitRange(o, SM, Lexer::getAsCharRange(StartEdge, SM, LangOpts), FM, indent + 1); Indent(o, indent) << "end\n"; - SourceLocation EndEdge = I->getEnd().asRange().getBegin(); - EmitRange(o, SM, LangOpts, CharSourceRange::getTokenRange(EndEdge), FM, + SourceRange EndEdge(SM.getExpansionLoc(I->getEnd().asRange().getBegin())); + EmitRange(o, SM, Lexer::getAsCharRange(EndEdge, SM, LangOpts), FM, indent + 1); --indent; @@ -154,7 +155,7 @@ static void ReportEvent(raw_ostream &o, const PathDiagnosticPiece& P, FullSourceLoc L = P.getLocation().asLocation(); Indent(o, indent) << "location\n"; - EmitLocation(o, SM, LangOpts, L, FM, indent); + EmitLocation(o, SM, L, FM, indent); // Output the ranges (if any). ArrayRef Ranges = P.getRanges(); @@ -163,11 +164,10 @@ static void ReportEvent(raw_ostream &o, const PathDiagnosticPiece& P, Indent(o, indent) << "ranges\n"; Indent(o, indent) << "\n"; ++indent; - for (ArrayRef::iterator I = Ranges.begin(), E = Ranges.end(); - I != E; ++I) { - EmitRange(o, SM, LangOpts, CharSourceRange::getTokenRange(*I), FM, - indent + 1); - } + for (auto &R : Ranges) + EmitRange(o, SM, + Lexer::getAsCharRange(SM.getExpansionRange(R), SM, LangOpts), + FM, indent + 1); --indent; Indent(o, indent) << "\n"; } @@ -453,7 +453,7 @@ void PlistDiagnostics::FlushDiagnosticsImpl( // Output the location of the bug. o << " location\n"; - EmitLocation(o, *SM, LangOpts, D->getLocation().asLocation(), FM, 2); + EmitLocation(o, *SM, D->getLocation().asLocation(), FM, 2); // Output the diagnostic to the sub-diagnostic client, if any. if (!filesMade->empty()) { diff --git a/clang/test/Analysis/diagnostics/report-issues-within-main-file.cpp b/clang/test/Analysis/diagnostics/report-issues-within-main-file.cpp index ec8106f..bba15f5 100644 --- a/clang/test/Analysis/diagnostics/report-issues-within-main-file.cpp +++ b/clang/test/Analysis/diagnostics/report-issues-within-main-file.cpp @@ -542,7 +542,7 @@ void callInMacroArg() { // CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: line69 -// CHECK-NEXT: col18 +// CHECK-NEXT: col51 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: diff --git a/clang/test/Analysis/plist-macros.cpp b/clang/test/Analysis/plist-macros.cpp index 0e8518a..58df569 100644 --- a/clang/test/Analysis/plist-macros.cpp +++ b/clang/test/Analysis/plist-macros.cpp @@ -903,7 +903,7 @@ void test2(int *p) { // CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: line45 -// CHECK-NEXT: col18 +// CHECK-NEXT: col21 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: @@ -1395,7 +1395,7 @@ void test2(int *p) { // CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: line82 -// CHECK-NEXT: col9 +// CHECK-NEXT: col12 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: @@ -1424,7 +1424,7 @@ void test2(int *p) { // CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: line82 -// CHECK-NEXT: col9 +// CHECK-NEXT: col12 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: -- 2.7.4