[PATCH] D16012: Carry raw string literal information through to the AST StringLiteral representation

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 12 13:48:18 PST 2016


rsmith added a comment.

In http://reviews.llvm.org/D16012#351821, @aaron.ballman wrote:

> Ping. Richard, I think we've provided justification that warrants this functionality (mostly for clang-tidy checks).


I am not convinced. This flag does not seem to make sense for `StringLiteral`, because it is not a property of the AST-level `StringLiteral` object; it is a property of the underlying tokens instead (and in general the `StringLiteral` object corresponds to multiple underlying tokens with differing rawness). We do not store other spelling information from the underlying tokens, such as whether a character was written literally or with an escape sequence, and in AST printing we do not preserve that; I do not see why this case should be any different. (We do store the translated value of the string, but that is a different case, both because it's part of the semantic model of the expression -- we don't want IR generation doing semantic string literal analysis -- and because it's needed frequently enough to justify caching it.)

I'm definitely sympathetic to making `StringLiteralParser` expose information on whether each token in the string literal is a raw string literal, and if so what prefix it uses. I can also see an argument for exposing an easier interface for constructing a `StringLiteralParser` from an existing `StringLiteral` object (note that `StringLiteral::getLocationOfByte` already does this dance). But I don't see that there's a compelling reason to break out and cache this information separately from its actual point of truth.

If we want to make AST printing round-trip string literals as-written, we should make it re-lex the string literal tokens and print them out verbatim -- making this work for raw vs non-raw strings but not for escapes versus non-escapes, or different flavours of escape sequence, doesn't make a lot of sense to me.


http://reviews.llvm.org/D16012





More information about the cfe-commits mailing list