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

[Zach Wheeler]

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.

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
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110627/9c82c2dd/attachment.html>