r243260 - Rename highly-confusing isWhitespace that returns false on \n to

Craig Topper craig.topper at gmail.com
Mon Jul 27 11:40:54 PDT 2015


On Sun, Jul 26, 2015 at 11:35 PM, Yaron Keren <yaron.keren at gmail.com> wrote:

> Author: yrnkrn
> Date: Mon Jul 27 01:35:22 2015
> New Revision: 243260
>
> URL: http://llvm.org/viewvc/llvm-project?rev=243260&view=rev
> Log:
> Rename highly-confusing isWhitespace that returns false on \n to
> isWhitespaceExceptNL. If that's wasn't bad enough, we have an identically
> named function in Basic/CharInfo.h.
>
> While at it, remove unnecessary includes, .str() calls and #ifdef.
>
>
> Modified:
>     cfe/trunk/lib/Rewrite/Rewriter.cpp
>
> Modified: cfe/trunk/lib/Rewrite/Rewriter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Rewrite/Rewriter.cpp?rev=243260&r1=243259&r2=243260&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Rewrite/Rewriter.cpp (original)
> +++ cfe/trunk/lib/Rewrite/Rewriter.cpp Mon Jul 27 01:35:22 2015
> @@ -15,11 +15,9 @@
>  #include "clang/Rewrite/Core/Rewriter.h"
>  #include "clang/Basic/Diagnostic.h"
>  #include "clang/Basic/DiagnosticIDs.h"
> -#include "clang/Basic/FileManager.h"
>  #include "clang/Basic/SourceManager.h"
>  #include "clang/Lex/Lexer.h"
>  #include "llvm/ADT/SmallString.h"
> -#include "llvm/Config/llvm-config.h"
>  #include "llvm/Support/FileSystem.h"
>  #include "llvm/Support/raw_ostream.h"
>  using namespace clang;
> @@ -33,9 +31,10 @@ raw_ostream &RewriteBuffer::write(raw_os
>    return os;
>  }
>
> +namespace {
>  /// \brief Return true if this character is non-new-line whitespace:
>  /// ' ', '\\t', '\\f', '\\v', '\\r'.
> -static inline bool isWhitespace(unsigned char c) {
> +inline bool isWhitespaceExceptNL(unsigned char c) {
>    switch (c) {
>    case ' ':
>    case '\t':
> @@ -47,6 +46,7 @@ static inline bool isWhitespace(unsigned
>      return false;
>    }
>  }
> +}
>

Don't the coding standards say to only use anonymous namespaces for class
definitions?


>
>  void RewriteBuffer::RemoveText(unsigned OrigOffset, unsigned Size,
>                                 bool removeLineIfEmpty) {
> @@ -80,7 +80,7 @@ void RewriteBuffer::RemoveText(unsigned
>
>      unsigned lineSize = 0;
>      posI = curLineStart;
> -    while (posI != end() && isWhitespace(*posI)) {
> +    while (posI != end() && isWhitespaceExceptNL(*posI)) {
>        ++posI;
>        ++lineSize;
>      }
> @@ -256,7 +256,7 @@ bool Rewriter::InsertText(SourceLocation
>      StringRef indentSpace;
>      {
>        unsigned i = lineOffs;
> -      while (isWhitespace(MB[i]))
> +      while (isWhitespaceExceptNL(MB[i]))
>          ++i;
>        indentSpace = MB.substr(lineOffs, i-lineOffs);
>      }
> @@ -363,12 +363,12 @@ bool Rewriter::IncreaseIndentation(CharS
>    StringRef parentSpace, startSpace;
>    {
>      unsigned i = parentLineOffs;
> -    while (isWhitespace(MB[i]))
> +    while (isWhitespaceExceptNL(MB[i]))
>        ++i;
>      parentSpace = MB.substr(parentLineOffs, i-parentLineOffs);
>
>      i = startLineOffs;
> -    while (isWhitespace(MB[i]))
> +    while (isWhitespaceExceptNL(MB[i]))
>        ++i;
>      startSpace = MB.substr(startLineOffs, i-startLineOffs);
>    }
> @@ -384,7 +384,7 @@ bool Rewriter::IncreaseIndentation(CharS
>    for (unsigned lineNo = startLineNo; lineNo <= endLineNo; ++lineNo) {
>      unsigned offs = Content->SourceLineCache[lineNo];
>      unsigned i = offs;
> -    while (isWhitespace(MB[i]))
> +    while (isWhitespaceExceptNL(MB[i]))
>        ++i;
>      StringRef origIndent = MB.substr(offs, i-offs);
>      if (origIndent.startswith(startSpace))
> @@ -409,7 +409,7 @@ public:
>      TempFilename = Filename;
>      TempFilename += "-%%%%%%%%";
>      int FD;
> -    if (llvm::sys::fs::createUniqueFile(TempFilename.str(), FD,
> TempFilename)) {
> +    if (llvm::sys::fs::createUniqueFile(TempFilename, FD, TempFilename)) {
>        AllWritten = false;
>        Diagnostics.Report(clang::diag::err_unable_to_make_temp)
>          << TempFilename;
> @@ -421,19 +421,15 @@ public:
>    ~AtomicallyMovedFile() {
>      if (!ok()) return;
>
> -    FileStream->flush();
> -#ifdef LLVM_ON_WIN32
> -    // Win32 does not allow rename/removing opened files.
> -    FileStream.reset();
> -#endif
> -    if (std::error_code ec =
> -            llvm::sys::fs::rename(TempFilename.str(), Filename)) {
> +    // Close (will also flush) theFileStream.
> +    FileStream->close();
> +    if (std::error_code ec = llvm::sys::fs::rename(TempFilename,
> Filename)) {
>        AllWritten = false;
>        Diagnostics.Report(clang::diag::err_unable_to_rename_temp)
>          << TempFilename << Filename << ec.message();
>        // If the remove fails, there's not a lot we can do - this is
> already an
>        // error.
> -      llvm::sys::fs::remove(TempFilename.str());
> +      llvm::sys::fs::remove(TempFilename);
>      }
>    }
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>



-- 
~Craig
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150727/86ec747b/attachment.html>


More information about the cfe-commits mailing list