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

[Zach Wheeler]

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

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<http://clang.llvm.org/doxygen/classclang_1_1Preprocessor.html#ab6fec9812af95c3c0033ca180fc98ad0>-- 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/dc625461/attachment.html>