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

[Yaron Keren]

Yes, you are correct, I actually remembered (or have seen code examples?)
the opposite and thus made the change.
Will fix this tomorrow, thanks.


2015-07-27 21:40 GMT+03:00 Craig Topper <craig.topper at gmail.com>:

>
>
> 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/20150728/d8d2e95a/attachment.html>