[cfe-commits] [PATCH] StringRef'ize API: clang::Token::getName()
Zach Wheeler
zachw.foss at gmail.com
Tue Jun 28 14:07:15 PDT 2011
Attached is an updated patch to StringRef'ize Token::getName() and
tok::getTokenName(), that incorporates Argyrios' comment.
@jahanian: Tok.getName().data() would do it for sure, of course; I don't
know about Tok.getName() directly.
ZJ
On Tue, Jun 28, 2011 at 4:02 PM, jahanian <fjahanian at apple.com> wrote:
>
> 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<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/0f820c01/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 3.diff
Type: application/octet-stream
Size: 2498 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110628/0f820c01/attachment.obj>
More information about the cfe-commits
mailing list