From 635423e61869a0997ec2eb853c1e1c92eaa8352d Mon Sep 17 00:00:00 2001 From: Eric Liu Date: Thu, 28 Apr 2016 07:52:03 +0000 Subject: [PATCH] Addressed reviewer's post-submission comments from http://reviews.llvm.org/D18551. Summary: Make SourceManager in Environment, WhitespaceManager, and FormatTokenAnalyzer etc constant members. Reviewers: djasper, klimek Subscribers: cfe-commits, klimek Differential Revision: http://reviews.llvm.org/D19587 llvm-svn: 267859 --- clang/lib/Format/AffectedRangeManager.h | 4 +- clang/lib/Format/ContinuationIndenter.cpp | 2 +- clang/lib/Format/ContinuationIndenter.h | 5 +- clang/lib/Format/Format.cpp | 116 ++++++++++------------ clang/lib/Format/WhitespaceManager.h | 4 +- 5 files changed, 61 insertions(+), 70 deletions(-) diff --git a/clang/lib/Format/AffectedRangeManager.h b/clang/lib/Format/AffectedRangeManager.h index 88b4dcfe76ff..1f7769432b49 100644 --- a/clang/lib/Format/AffectedRangeManager.h +++ b/clang/lib/Format/AffectedRangeManager.h @@ -25,7 +25,7 @@ class AnnotatedLine; class AffectedRangeManager { public: - AffectedRangeManager(SourceManager &SourceMgr, + AffectedRangeManager(const SourceManager &SourceMgr, const ArrayRef Ranges) : SourceMgr(SourceMgr), Ranges(Ranges.begin(), Ranges.end()) {} @@ -57,7 +57,7 @@ private: bool nonPPLineAffected(AnnotatedLine *Line, const AnnotatedLine *PreviousLine); - SourceManager &SourceMgr; + const SourceManager &SourceMgr; const SmallVector Ranges; }; diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index bfcb8e562e3b..f4d5ba62035d 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -64,7 +64,7 @@ static bool startsNextParameter(const FormatToken &Current, ContinuationIndenter::ContinuationIndenter(const FormatStyle &Style, const AdditionalKeywords &Keywords, - SourceManager &SourceMgr, + const SourceManager &SourceMgr, WhitespaceManager &Whitespaces, encoding::Encoding Encoding, bool BinPackInconclusiveFunctions) diff --git a/clang/lib/Format/ContinuationIndenter.h b/clang/lib/Format/ContinuationIndenter.h index 9b9154ed3095..21ad653c4fa4 100644 --- a/clang/lib/Format/ContinuationIndenter.h +++ b/clang/lib/Format/ContinuationIndenter.h @@ -38,7 +38,8 @@ public: /// column \p FirstIndent. ContinuationIndenter(const FormatStyle &Style, const AdditionalKeywords &Keywords, - SourceManager &SourceMgr, WhitespaceManager &Whitespaces, + const SourceManager &SourceMgr, + WhitespaceManager &Whitespaces, encoding::Encoding Encoding, bool BinPackInconclusiveFunctions); @@ -137,7 +138,7 @@ private: FormatStyle Style; const AdditionalKeywords &Keywords; - SourceManager &SourceMgr; + const SourceManager &SourceMgr; WhitespaceManager &Whitespaces; encoding::Encoding Encoding; bool BinPackInconclusiveFunctions; diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 6370be903dde..efcecee1401c 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -784,7 +784,7 @@ namespace { class FormatTokenLexer { public: - FormatTokenLexer(SourceManager &SourceMgr, FileID ID, + FormatTokenLexer(const SourceManager &SourceMgr, FileID ID, const FormatStyle &Style, encoding::Encoding Encoding) : FormatTok(nullptr), IsFirstToken(true), GreaterStashed(false), LessStashed(false), Column(0), TrailingWhitespace(0), @@ -1373,7 +1373,7 @@ private: unsigned Column; unsigned TrailingWhitespace; std::unique_ptr Lex; - SourceManager &SourceMgr; + const SourceManager &SourceMgr; FileID ID; const FormatStyle &Style; IdentifierTable IdentTable; @@ -1451,25 +1451,21 @@ static StringRef getLanguageName(FormatStyle::LanguageKind Language) { class Environment { public: - Environment(const FormatStyle &Style, SourceManager &SM, FileID ID, - ArrayRef Ranges) - : Style(Style), ID(ID), CharRanges(Ranges.begin(), Ranges.end()), SM(SM) { - } + Environment(SourceManager &SM, FileID ID, ArrayRef Ranges) + : ID(ID), CharRanges(Ranges.begin(), Ranges.end()), SM(SM) {} - Environment(const FormatStyle &Style, FileID ID, - std::unique_ptr FileMgr, + Environment(FileID ID, std::unique_ptr FileMgr, std::unique_ptr VirtualSM, std::unique_ptr Diagnostics, const std::vector &CharRanges) - : Style(Style), ID(ID), CharRanges(CharRanges.begin(), CharRanges.end()), + : ID(ID), CharRanges(CharRanges.begin(), CharRanges.end()), SM(*VirtualSM), FileMgr(std::move(FileMgr)), VirtualSM(std::move(VirtualSM)), Diagnostics(std::move(Diagnostics)) {} // This sets up an virtual file system with file \p FileName containing \p // Code. static std::unique_ptr - CreateVirtualEnvironment(const FormatStyle &Style, StringRef Code, - StringRef FileName, + CreateVirtualEnvironment(StringRef Code, StringRef FileName, ArrayRef Ranges) { // This is referenced by `FileMgr` and will be released by `FileMgr` when it // is deleted. @@ -1501,25 +1497,20 @@ public: SourceLocation End = Start.getLocWithOffset(Range.getLength()); CharRanges.push_back(CharSourceRange::getCharRange(Start, End)); } - return llvm::make_unique(Style, ID, std::move(FileMgr), + return llvm::make_unique(ID, std::move(FileMgr), std::move(VirtualSM), std::move(Diagnostics), CharRanges); } - FormatStyle &getFormatStyle() { return Style; } - - const FormatStyle &getFormatStyle() const { return Style; } - FileID getFileID() const { return ID; } StringRef getFileName() const { return FileName; } ArrayRef getCharRanges() const { return CharRanges; } - SourceManager &getSourceManager() { return SM; } + const SourceManager &getSourceManager() const { return SM; } private: - FormatStyle Style; FileID ID; StringRef FileName; SmallVector CharRanges; @@ -1535,8 +1526,9 @@ private: class TokenAnalyzer : public UnwrappedLineConsumer { public: - TokenAnalyzer(Environment &Env) - : Env(Env), AffectedRangeMgr(Env.getSourceManager(), Env.getCharRanges()), + TokenAnalyzer(const Environment &Env, const FormatStyle &Style) + : Style(Style), Env(Env), + AffectedRangeMgr(Env.getSourceManager(), Env.getCharRanges()), UnwrappedLines(1), Encoding(encoding::detectEncoding( Env.getSourceManager().getBufferData(Env.getFileID()))) { @@ -1544,18 +1536,17 @@ public: << (Encoding == encoding::Encoding_UTF8 ? "UTF8" : "unknown") << "\n"); - DEBUG(llvm::dbgs() << "Language: " - << getLanguageName(Env.getFormatStyle().Language) + DEBUG(llvm::dbgs() << "Language: " << getLanguageName(Style.Language) << "\n"); } tooling::Replacements process() { tooling::Replacements Result; - FormatTokenLexer Tokens(Env.getSourceManager(), Env.getFileID(), - Env.getFormatStyle(), Encoding); + FormatTokenLexer Tokens(Env.getSourceManager(), Env.getFileID(), Style, + Encoding); - UnwrappedLineParser Parser(Env.getFormatStyle(), Tokens.getKeywords(), - Tokens.lex(), *this); + UnwrappedLineParser Parser(Style, Tokens.getKeywords(), Tokens.lex(), + *this); Parser.parse(); assert(UnwrappedLines.rbegin()->empty()); for (unsigned Run = 0, RunE = UnwrappedLines.size(); Run + 1 != RunE; @@ -1563,7 +1554,7 @@ public: DEBUG(llvm::dbgs() << "Run " << Run << "...\n"); SmallVector AnnotatedLines; - TokenAnnotator Annotator(Env.getFormatStyle(), Tokens.getKeywords()); + TokenAnnotator Annotator(Style, Tokens.getKeywords()); for (unsigned i = 0, e = UnwrappedLines[Run].size(); i != e; ++i) { AnnotatedLines.push_back(new AnnotatedLine(UnwrappedLines[Run][i])); Annotator.annotate(*AnnotatedLines.back()); @@ -1603,8 +1594,9 @@ protected: UnwrappedLines.push_back(SmallVector()); } + FormatStyle Style; // Stores Style, FileID and SourceManager etc. - Environment &Env; + const Environment &Env; // AffectedRangeMgr stores ranges to be fixed. AffectedRangeManager AffectedRangeMgr; SmallVector, 2> UnwrappedLines; @@ -1613,8 +1605,9 @@ protected: class Formatter : public TokenAnalyzer { public: - Formatter(Environment &Env, bool *IncompleteFormat) - : TokenAnalyzer(Env), IncompleteFormat(IncompleteFormat) {} + Formatter(const Environment &Env, const FormatStyle &Style, + bool *IncompleteFormat) + : TokenAnalyzer(Env, Style), IncompleteFormat(IncompleteFormat) {} tooling::Replacements analyze(TokenAnnotator &Annotator, @@ -1624,8 +1617,8 @@ public: AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(), AnnotatedLines.end()); - if (Env.getFormatStyle().Language == FormatStyle::LK_JavaScript && - Env.getFormatStyle().JavaScriptQuotes != FormatStyle::JSQS_Leave) + if (Style.Language == FormatStyle::LK_JavaScript && + Style.JavaScriptQuotes != FormatStyle::JSQS_Leave) requoteJSStringLiteral(AnnotatedLines, Result); for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) { @@ -1635,13 +1628,13 @@ public: Annotator.setCommentLineLevels(AnnotatedLines); WhitespaceManager Whitespaces( - Env.getSourceManager(), Env.getFormatStyle(), + Env.getSourceManager(), Style, inputUsesCRLF(Env.getSourceManager().getBufferData(Env.getFileID()))); - ContinuationIndenter Indenter(Env.getFormatStyle(), Tokens.getKeywords(), + ContinuationIndenter Indenter(Style, Tokens.getKeywords(), Env.getSourceManager(), Whitespaces, Encoding, BinPackInconclusiveFunctions); - UnwrappedLineFormatter(&Indenter, &Whitespaces, Env.getFormatStyle(), - Tokens.getKeywords(), IncompleteFormat) + UnwrappedLineFormatter(&Indenter, &Whitespaces, Style, Tokens.getKeywords(), + IncompleteFormat) .format(AnnotatedLines); return Whitespaces.generateReplacements(); } @@ -1663,17 +1656,14 @@ private: // NB: testing for not starting with a double quote to avoid // breaking // `template strings`. - (Env.getFormatStyle().JavaScriptQuotes == - FormatStyle::JSQS_Single && + (Style.JavaScriptQuotes == FormatStyle::JSQS_Single && !Input.startswith("\"")) || - (Env.getFormatStyle().JavaScriptQuotes == - FormatStyle::JSQS_Double && + (Style.JavaScriptQuotes == FormatStyle::JSQS_Double && !Input.startswith("\'"))) continue; // Change start and end quote. - bool IsSingle = - Env.getFormatStyle().JavaScriptQuotes == FormatStyle::JSQS_Single; + bool IsSingle = Style.JavaScriptQuotes == FormatStyle::JSQS_Single; SourceLocation Start = FormatTok->Tok.getLocation(); auto Replace = [&](SourceLocation Start, unsigned Length, StringRef ReplacementText) { @@ -1785,14 +1775,14 @@ private: Tok = Tok->Next; } } - if (Env.getFormatStyle().DerivePointerAlignment) - Env.getFormatStyle().PointerAlignment = - countVariableAlignments(AnnotatedLines) <= 0 ? FormatStyle::PAS_Left - : FormatStyle::PAS_Right; - if (Env.getFormatStyle().Standard == FormatStyle::LS_Auto) - Env.getFormatStyle().Standard = hasCpp03IncompatibleFormat(AnnotatedLines) - ? FormatStyle::LS_Cpp11 - : FormatStyle::LS_Cpp03; + if (Style.DerivePointerAlignment) + Style.PointerAlignment = countVariableAlignments(AnnotatedLines) <= 0 + ? FormatStyle::PAS_Left + : FormatStyle::PAS_Right; + if (Style.Standard == FormatStyle::LS_Auto) + Style.Standard = hasCpp03IncompatibleFormat(AnnotatedLines) + ? FormatStyle::LS_Cpp11 + : FormatStyle::LS_Cpp03; BinPackInconclusiveFunctions = HasBinPackedFunction || !HasOnePerLineFunction; } @@ -1805,8 +1795,8 @@ private: // file. class Cleaner : public TokenAnalyzer { public: - Cleaner(Environment &Env) - : TokenAnalyzer(Env), + Cleaner(const Environment &Env, const FormatStyle &Style) + : TokenAnalyzer(Env, Style), DeletedTokens(FormatTokenLess(Env.getSourceManager())) {} // FIXME: eliminate unused parameters. @@ -1864,7 +1854,7 @@ private: bool checkEmptyNamespace(SmallVectorImpl &AnnotatedLines, unsigned CurrentLine, unsigned &NewLine) { unsigned InitLine = CurrentLine, End = AnnotatedLines.size(); - if (Env.getFormatStyle().BraceWrapping.AfterNamespace) { + if (Style.BraceWrapping.AfterNamespace) { // If the left brace is in a new line, we should consume it first so that // it does not make the namespace non-empty. // FIXME: error handling if there is no left brace. @@ -1949,13 +1939,13 @@ private: // We store tokens in the order they appear in the translation unit so that // we do not need to sort them in `generateFixes()`. struct FormatTokenLess { - FormatTokenLess(SourceManager &SM) : SM(SM) {} + FormatTokenLess(const SourceManager &SM) : SM(SM) {} bool operator()(const FormatToken *LHS, const FormatToken *RHS) { return SM.isBeforeInTranslationUnit(LHS->Tok.getLocation(), RHS->Tok.getLocation()); } - SourceManager &SM; + const SourceManager &SM; }; // Tokens to be deleted. @@ -2182,8 +2172,8 @@ tooling::Replacements reformat(const FormatStyle &Style, SourceManager &SM, if (Expanded.DisableFormat) return tooling::Replacements(); - Environment Env(Expanded, SM, ID, Ranges); - Formatter Format(Env, IncompleteFormat); + Environment Env(SM, ID, Ranges); + Formatter Format(Env, Expanded, IncompleteFormat); return Format.process(); } @@ -2195,15 +2185,15 @@ tooling::Replacements reformat(const FormatStyle &Style, StringRef Code, return tooling::Replacements(); std::unique_ptr Env = - Environment::CreateVirtualEnvironment(Expanded, Code, FileName, Ranges); - Formatter Format(*Env, IncompleteFormat); + Environment::CreateVirtualEnvironment(Code, FileName, Ranges); + Formatter Format(*Env, Expanded, IncompleteFormat); return Format.process(); } tooling::Replacements cleanup(const FormatStyle &Style, SourceManager &SM, FileID ID, ArrayRef Ranges) { - Environment Env(Style, SM, ID, Ranges); - Cleaner Clean(Env); + Environment Env(SM, ID, Ranges); + Cleaner Clean(Env, Style); return Clean.process(); } @@ -2211,8 +2201,8 @@ tooling::Replacements cleanup(const FormatStyle &Style, StringRef Code, ArrayRef Ranges, StringRef FileName) { std::unique_ptr Env = - Environment::CreateVirtualEnvironment(Style, Code, FileName, Ranges); - Cleaner Clean(*Env); + Environment::CreateVirtualEnvironment(Code, FileName, Ranges); + Cleaner Clean(*Env, Style); return Clean.process(); } diff --git a/clang/lib/Format/WhitespaceManager.h b/clang/lib/Format/WhitespaceManager.h index 9ca9db6f7488..3562347a0e60 100644 --- a/clang/lib/Format/WhitespaceManager.h +++ b/clang/lib/Format/WhitespaceManager.h @@ -37,7 +37,7 @@ namespace format { /// There may be multiple calls to \c breakToken for a given token. class WhitespaceManager { public: - WhitespaceManager(SourceManager &SourceMgr, const FormatStyle &Style, + WhitespaceManager(const SourceManager &SourceMgr, const FormatStyle &Style, bool UseCRLF) : SourceMgr(SourceMgr), Style(Style), UseCRLF(UseCRLF) {} @@ -203,7 +203,7 @@ private: unsigned Spaces, unsigned WhitespaceStartColumn); SmallVector Changes; - SourceManager &SourceMgr; + const SourceManager &SourceMgr; tooling::Replacements Replaces; const FormatStyle &Style; bool UseCRLF;