[cfe-commits] r79448 - in /cfe/trunk: include/clang/Rewrite/Rewriter.h lib/Frontend/FixItRewriter.cpp lib/Frontend/RewriteBlocks.cpp lib/Frontend/RewriteMacros.cpp lib/Frontend/RewriteObjC.cpp lib/Rewrite/HTMLRewrite.cpp lib/Rewrite/Rewriter.cpp

[Sebastian Redl]

Daniel Dunbar wrote:
> Author: ddunbar
> Date: Wed Aug 19 14:10:30 2009
> New Revision: 79448
>
> URL: http://llvm.org/viewvc/llvm-project?rev=79448&view=rev
> Log:
> Convert parts of Rewriter to StringRef based API.
>  - Please accept my sincere apologies for the gratuitous elimination of code
>    duplication, manual string length counting, unnecessary strlen calls, etc.
>   
How could you?
>    
>    /// InsertText - Insert some text at the specified point, where the offset in
>    /// the buffer is specified relative to the original SourceBuffer.  The
>    /// text is inserted after the specified location.  This is method is the
>    /// same as InsertText with "InsertAfter == false".
> -  void InsertTextAfter(unsigned OrigOffset, const char *StrData,
> -                       unsigned StrLen) {
> -    InsertText(OrigOffset, StrData, StrLen);
> +  void InsertTextAfter(unsigned OrigOffset, const llvm::StringRef &Str) {
> +    InsertText(OrigOffset, Str);
>    }
>   
This comment is wrong, both in function name and description (it's
insertAfter==true). Not part of your patch, actually, but since you're
in the area ...
> @@ -159,7 +158,7 @@
>    /// InsertText - Insert the specified string at the specified location in the
>    /// original buffer.  This method returns true (and does nothing) if the input
>    /// location was not rewritable, false otherwise.
> -  bool InsertText(SourceLocation Loc, const char *StrData, unsigned StrLen,
> +  bool InsertText(SourceLocation Loc, const llvm::StringRef &Str,
>                    bool InsertAfter = true);
>    
>    /// InsertTextAfter - Insert the specified string at the specified location in
> @@ -167,9 +166,8 @@
>    ///  the input location was not rewritable, false otherwise.  Text is
>    ///  inserted after any other text that has been previously inserted
>    ///  at the some point (the default behavior for InsertText).
> -  bool InsertTextAfter(SourceLocation Loc, const char *StrData,
> -                       unsigned StrLen) {
> -    return InsertText(Loc, StrData, StrLen);
> +  bool InsertTextAfter(SourceLocation Loc, const llvm::StringRef &Str) {
> +    return InsertText(Loc, Str, false);
>    }    
>   
This one looks  actually wrong in the implementation.

Sebastian