[cfe-commits] [PATCH] StringRef'ize API: clang::Token::getName()

[jahanian]

On Jun 28, 2011, at 12:50 PM, Zach Wheeler wrote:

> Would it be at all useful to you for it to return StringRef?

As long as something like  "% p Tok.getName()" displays the token in readable form somewhere in StringRef
container, it should be OK.

- Fariborz

> 
> ZJ
> 
> On Tue, Jun 28, 2011 at 3:28 PM, jahanian <fjahanian at apple.com> wrote:
> 
> On Jun 28, 2011, at 11:43 AM, Zach Wheeler wrote:
> 
>> Below is a patch to kill Token::getName(). (Would it be better to attach patches rather than cut and paste them?)
> 
> I frequently call it in the debugger to get the Token name. We should have something in its
> place before killing it.
> 
> - Fariborz
> 
>> 
>> Clang built without complaint regarding this change.
>> 
>> 
>> 
>> Index: Token.h
>> ===================================================================
>> --- Token.h	(revision 133884)
>> +++ Token.h	(working copy)
>> @@ -145,10 +145,6 @@
>>      setAnnotationEndLoc(R.getEnd());
>>    }
>>  
>> -  const char *getName() const {
>> -    return tok::getTokenName( (tok::TokenKind) Kind);
>> -  }
>> -
>>    /// startToken - Reset all flags to cleared.
>>    ///
>>    void startToken() {
>> 
>> 
>> On Tue, Jun 28, 2011 at 12:03 AM, Chris Lattner <clattner at apple.com> wrote:
>> 
>> On Jun 27, 2011, at 7:56 PM, Zach Wheeler wrote:
>> 
>>> Token::getName() doesn't appear to be used presently, but it could (and perhaps should) be used on line 159 of lib/Lex/Preprocessor.cpp -- use "Tok.getName()" instead of "tok::getTokenName(Tok.getKind())".
>>> 
>>> Token::getName() just calls tok::getTokenName(); perhaps the former should return StringRef (since Token is an abstraction), and the latter should return const char* (since tok::getTokenName() is merely a procedure which returns a string constant)?
>>> 
>>> Maybe this particular issue doesn't matter enough to worry about all this though.
>> 
>> Changing TokenKinds.h to return a stringref isn't compelling to me, but I'm not opposed to it.  Changing the method in Token.h requires moving it out of line, adding the #include, or *removing it*.  I vote for the later :)
>> 
>> -Chris
>> 
>>> 
>>> Thanks for your patience. :-)
>>> 
>>> ZJ
>>> 
>>> On Mon, Jun 27, 2011 at 8:14 PM, Chris Lattner <clattner at apple.com> wrote:
>>> 
>>> On Jun 25, 2011, at 2:55 PM, Zach Wheeler wrote:
>>> 
>>>> This patch is intended to change clang::Token::getName()  so that it returns llvm::StringRef instead of const char*.
>>>> 
>>>> This is my first patch (ever), so if I did something I wasn't supposed to, just scream at me and I'll try to fix it. :-)
>>>> 
>>>> This turned out to be an easy place to start; the doxygen reference indicates that this method isn't referenced at all. Clang built fine with these changes.
>>> 
>>> Hi Zach,
>>> 
>>> This is a great start, but I'd prefer to not change these: if they aren't called at all, please send in a patch to nuke them :).  Also, since these are just returning constant C strings, a "const char*" is actually good enough.  The places that we'd like to convert to StringRef are ones that are manually implementing the same thing (by taking two const char*'s, or a pointer+size).
>>> 
>>> Thanks again for working on this area!
>>> 
>>> -Chris
>>> 
>>>> 
>>>> Regards, 
>>>> ZJ
>>>> 
>>>> 
>>>> 
>>>> Index: include/clang/Basic/TokenKinds.h
>>>> ===================================================================
>>>> --- TokenKinds.h	(revision 133818)
>>>> +++ TokenKinds.h	(working copy)
>>>> @@ -14,6 +14,10 @@
>>>>  #ifndef LLVM_CLANG_TOKENKINDS_H
>>>>  #define LLVM_CLANG_TOKENKINDS_H
>>>>  
>>>> +namespace llvm {
>>>> +  class StringRef;
>>>> +}
>>>> +
>>>>  namespace clang {
>>>>  
>>>>  namespace tok {
>>>> @@ -53,7 +57,7 @@
>>>>  ///
>>>>  /// The name of a token will be an internal name (such as "l_square")
>>>>  /// and should not be used as part of diagnostic messages.
>>>> -const char *getTokenName(enum TokenKind Kind);
>>>> +llvm::StringRef getTokenName(enum TokenKind Kind);
>>>>  
>>>>  /// \brief Determines the spelling of simple punctuation tokens like
>>>>  /// '!' or '%', and returns NULL for literal and annotation tokens.
>>>> 
>>>> Index: lib/Basic/TokenKinds.cpp
>>>> ===================================================================
>>>> --- TokenKinds.cpp	(revision 133818)
>>>> +++ TokenKinds.cpp	(working copy)
>>>> @@ -12,7 +12,7 @@
>>>>  //===----------------------------------------------------------------------===//
>>>>  
>>>>  #include "clang/Basic/TokenKinds.h"
>>>> -
>>>> +#include "llvm/ADT/StringRef.h"
>>>>  #include <cassert>
>>>>  using namespace clang;
>>>>  
>>>> @@ -23,9 +23,9 @@
>>>>    0
>>>>  };
>>>>  
>>>> -const char *tok::getTokenName(enum TokenKind Kind) {
>>>> +llvm::StringRef tok::getTokenName(enum TokenKind Kind) {
>>>>    assert(Kind < tok::NUM_TOKENS);
>>>> -  return TokNames[Kind];
>>>> +  return llvm::StringRef(TokNames[Kind]);
>>>>  }
>>>>  
>>>>  const char *tok::getTokenSimpleSpelling(enum TokenKind Kind) {
>>>> 
>>>> Index: include/clang/Lex/Token.h
>>>> ===================================================================
>>>> --- Token.h	(revision 133818)
>>>> +++ Token.h	(working copy)
>>>> @@ -18,6 +18,7 @@
>>>>  #include "clang/Basic/TokenKinds.h"
>>>>  #include "clang/Basic/SourceLocation.h"
>>>>  #include "clang/Basic/OperatorKinds.h"
>>>> +#include "llvm/ADT/StringRef.h"
>>>>  #include <cstdlib>
>>>>  
>>>>  namespace clang {
>>>> @@ -145,7 +146,7 @@
>>>>      setAnnotationEndLoc(R.getEnd());
>>>>    }
>>>>  
>>>> -  const char *getName() const {
>>>> +  llvm::StringRef getName() const {
>>>>      return tok::getTokenName( (tok::TokenKind) Kind);
>>>>    }
>>>>  
>>>> 
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>> 
>>> 
>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110628/2be9c015/attachment.html>