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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to