Carry raw string literal information through to the AST StringLiteral representation

[aaron.ballman]

Summary

The AST does not currently carry information about whether a string literal is raw or not in the StringLiteral AST representation, which this patch rectifies. It exposes information about whether a string literal is raw, and what prefix (if any) a raw string literal uses. The patch also adds support for pretty printing a raw string literal.

One thing to note, however, is that this is not 100% perfect because of translation phase 6, where adjacent string literals are concatenated. This patch only supports concatenated raw string literals that have the same prefix. It does not support differing prefixes, or adjacent raw & nonraw literals. I felt that these were a sufficiently uncommon edge case to not attempt to support.

[aaron.ballman]

Ping

[aaron.ballman]

In D16012#331219, @aaron.ballman wrote:

Ping

Ping

[rsmith]

What's the benefit of storing this? You can get the same effect by
re-lexing. We don't guarantee that the pretty printed version of the AST
comprises the same sequence of tokens in general.

[LegalizeAdulthood]

In D16012#337208, @rsmith wrote:

What's the benefit of storing this? You can get the same effect by
re-lexing. We don't guarantee that the pretty printed version of the AST
comprises the same sequence of tokens in general.

In writing clang-tidy checks, I've had to do re-lexing a number of times and personally I find it to be a complete PITA and very easy to get wrong, plus it results in many review iterations because of all the ways that StringRef enters the picture, whether you're using the plain lexer or the raw lexer, SourceRange or CharSourceRange, yadda yadda yadda.

I thought the whole point of the AST was to do this work once and store the results of the work for tools.

[aaron.ballman]

Ping. Richard, I think we've provided justification that warrants this functionality (mostly for clang-tidy checks). Any issues with the code?

[rsmith]

In 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.

[aaron.ballman]

Your logic makes sense to me. I am abandoning this revision while I think about alternatives. Thanks!

[LegalizeAdulthood]

In D16012#351827, @rsmith wrote:

I'm definitely sympathetic to making StringLiteralParser expose information [...]

I was unaware of this class; so far I have only studied the classes appearing in the AST.

I did notice that the AST shows string literals after concatenation (which makes perfect sense) and sometimes for refactoring you want to treat the individual string literal chunks separately. My clang-tidy check for raw string literals is probably confused by literal concatenation in the AST, so I will go add some tests for those cases.

One area of clang-tidy/refactoring tooling code development that is pretty opaque at the moment is when you should dip down to relexing/reparsing the source text and when you use the AST. So far most (all?) of the documentation on guiding developers to writing such tools talks almost exclusively about the AST. Anything we can do to simplify lexing/parsing tasks when you need to dip lower than the AST would be most welcome.

include/clang/AST/Expr.h

     const uint32_t *asUInt32;
   } StrData;
   unsigned Length;
-  unsigned CharByteWidth : 4;
+  unsigned CharByteWidth : 3;
   unsigned Kind : 3;
   unsigned IsPascal : 1;
+  unsigned IsRaw : 1;
   unsigned NumConcatenated;
+  const char *RawPrefix;
   SourceLocation TokLocs[1];
 
   StringLiteral(QualType Ty) :

   /// This is the "fully general" constructor that allows representation of
   /// strings formed from multiple concatenated tokens.
   static StringLiteral *Create(const ASTContext &C, StringRef Str,
-                               StringKind Kind, bool Pascal, QualType Ty,
+                               StringKind Kind, bool Pascal, bool Raw,
+                               StringRef RawPrefix, QualType Ty,
                                const SourceLocation *Loc, unsigned NumStrs);
 
   /// Simple constructor for string literals made from one token.
   static StringLiteral *Create(const ASTContext &C, StringRef Str,
-                               StringKind Kind, bool Pascal, QualType Ty,
+                               StringKind Kind, bool Pascal, bool Raw,
+                               StringRef RawPrefix, QualType Ty,
                                SourceLocation Loc) {
-    return Create(C, Str, Kind, Pascal, Ty, &Loc, 1);
+    return Create(C, Str, Kind, Pascal, Raw, RawPrefix, Ty, &Loc, 1);
   }
 
   /// \brief Construct an empty string literal.

 
   /// \brief Sets the string data to the given string data.
   void setString(const ASTContext &C, StringRef Str,
-                 StringKind Kind, bool IsPascal);
+                 StringKind Kind, bool IsPascal, bool IsRaw,
+                 StringRef RawPrefix);
 
   StringKind getKind() const { return static_cast<StringKind>(Kind); }
 

   bool isUTF32() const { return Kind == UTF32; }
   bool isPascal() const { return IsPascal; }
 
+  /// \brief A string literal is considered to be a raw string literal only when
+  // all of the string tokens (before translation stage 6 concatenation) are raw
+  // strings with the same prefix. So, for instance:
+  // R"(a)" R"(b)" -> R"(ab)", R"test(a)test" R"test(b)test" -> R"test(ab)test"
+  // R"test(a)test" R"ing(b)ing" -> "ab". FIXME: This is unfortunate because we
+  // lose information in the AST.
+  bool isRaw() const { return IsRaw; }
+
+  /// \brief Gets the prefix used when creating a raw string literal; note that
+  /// the prefix is optional, so this may return an empty StringRef.
+  StringRef getRawPrefix() const {
+    return RawPrefix ? StringRef(RawPrefix) : StringRef();
+  }
+
   bool containsNonAsciiOrNull() const {
     StringRef Str = getString();
     for (unsigned i = 0, e = Str.size(); i != e; ++i)

include/clang/Lex/LiteralSupport.h

   SmallString<32> UDSuffixBuf;
   unsigned UDSuffixToken;
   unsigned UDSuffixOffset;
+  SmallString<8> RawPrefix;
+
 public:
   StringLiteralParser(ArrayRef<Token> StringToks,
                       Preprocessor &PP, bool Complain = true);

     
 
   bool hadError;
-  bool Pascal;
+  bool Pascal, Raw;
 
+  StringRef GetRawStringPrefix() const {
+    return StringRef(RawPrefix.data(), RawPrefix.size());
+  }
+
   StringRef GetString() const {
     return StringRef(ResultBuf.data(), GetStringLength());
   }

lib/AST/Expr.cpp

 }
 
 StringLiteral *StringLiteral::Create(const ASTContext &C, StringRef Str,
-                                     StringKind Kind, bool Pascal, QualType Ty,
+                                     StringKind Kind, bool Pascal, bool Raw,
+                                     StringRef RawPrefix, QualType Ty,
                                      const SourceLocation *Loc,
                                      unsigned NumStrs) {
   assert(C.getAsConstantArrayType(Ty) &&

   StringLiteral *SL = new (Mem) StringLiteral(Ty);
 
   // OPTIMIZE: could allocate this appended to the StringLiteral.
-  SL->setString(C,Str,Kind,Pascal);
+  SL->setString(C, Str, Kind, Pascal, Raw, RawPrefix);
 
   SL->TokLocs[0] = Loc[0];
   SL->NumConcatenated = NumStrs;

   SL->CharByteWidth = 0;
   SL->Length = 0;
   SL->NumConcatenated = NumStrs;
+  SL->RawPrefix = nullptr;
+  SL->IsRaw = false;
   return SL;
 }
 

   case UTF16: OS << 'u'; break;
   case UTF32: OS << 'U'; break;
   }
+
+  if (IsRaw)
+    OS << "R";
   OS << '"';
+  if (RawPrefix)
+    OS << RawPrefix;  
+  if (IsRaw)
+    OS << "(";
+
   static const char Hex[] = "0123456789ABCDEF";
 
   unsigned LastSlashX = getLength();
   for (unsigned I = 0, N = getLength(); I != N; ++I) {
-    switch (uint32_t Char = getCodeUnit(I)) {
-    default:
-      // FIXME: Convert UTF-8 back to codepoints before rendering.
+    if (IsRaw) {
+      // For raw strings, print the contents directly to the stream without
+      // converting to a more human-readable format.
+      OS << (char)getCodeUnit(I);
+    } else {
+      switch (uint32_t Char = getCodeUnit(I)) {
+      default:
+        // FIXME: Convert UTF-8 back to codepoints before rendering.
 
         // Convert UTF-16 surrogate pairs back to codepoints before rendering.
         // Leave invalid surrogates alone; we'll use \x for those.
-      if (getKind() == UTF16 && I != N - 1 && Char >= 0xd800 && 
+        if (getKind() == UTF16 && I != N - 1 && Char >= 0xd800 &&
             Char <= 0xdbff) {
           uint32_t Trail = getCodeUnit(I + 1);
           if (Trail >= 0xdc00 && Trail <= 0xdfff) {
             Char = 0x10000 + ((Char - 0xd800) << 10) + (Trail - 0xdc00);
             ++I;
+          }
         }
-      }
 
         if (Char > 0xff) {
           // If this is a wide string, output characters over 0xff using \x
-        // escapes. Otherwise, this is a UTF-16 or UTF-32 string, and Char is a
-        // codepoint: use \x escapes for invalid codepoints.
+          // escapes. Otherwise, this is a UTF-16 or UTF-32 string, and Char is
+          // a codepoint: use \x escapes for invalid codepoints.
           if (getKind() == Wide ||
               (Char >= 0xd800 && Char <= 0xdfff) || Char >= 0x110000) {
             // FIXME: Is this the best way to print wchar_t?
             OS << "\\x";
             int Shift = 28;
             while ((Char >> Shift) == 0)
               Shift -= 4;
             for (/**/; Shift >= 0; Shift -= 4)
               OS << Hex[(Char >> Shift) & 15];
             LastSlashX = I;
+            break;
+          }
+
+          if (Char > 0xffff)
+            OS << "\\U00"
+            << Hex[(Char >> 20) & 15]
+            << Hex[(Char >> 16) & 15];
+          else
+            OS << "\\u";
+          OS << Hex[(Char >> 12) & 15]
+            << Hex[(Char >> 8) & 15]
+            << Hex[(Char >> 4) & 15]
+            << Hex[(Char >> 0) & 15];
           break;
         }
 
-        if (Char > 0xffff)
-          OS << "\\U00"
-             << Hex[(Char >> 20) & 15]
-             << Hex[(Char >> 16) & 15];
-        else
-          OS << "\\u";
-        OS << Hex[(Char >> 12) & 15]
-           << Hex[(Char >>  8) & 15]
-           << Hex[(Char >>  4) & 15]
-           << Hex[(Char >>  0) & 15];
-        break;
-      }
-
-      // If we used \x... for the previous character, and this character is a
-      // hexadecimal digit, prevent it being slurped as part of the \x.
-      if (LastSlashX + 1 == I) {
-        switch (Char) {
+        // If we used \x... for the previous character, and this character is a
+        // hexadecimal digit, prevent it being slurped as part of the \x.
+        if (LastSlashX + 1 == I) {
+          switch (Char) {
           case '0': case '1': case '2': case '3': case '4':
           case '5': case '6': case '7': case '8': case '9':
           case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
           case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
             OS << "\"\"";
+          }
         }
-      }
 
         assert(Char <= 0xff &&
                "Characters above 0xff should already have been handled.");
 
         if (isPrintable(Char))
           OS << (char)Char;
         else  // Output anything hard as an octal escape.
           OS << '\\'
-           << (char)('0' + ((Char >> 6) & 7))
-           << (char)('0' + ((Char >> 3) & 7))
-           << (char)('0' + ((Char >> 0) & 7));
         break;
       // Handle some common non-printable cases to make dumps prettier.
       case '\\': OS << "\\\\"; break;
       case '"': OS << "\\\""; break;
       case '\n': OS << "\\n"; break;
       case '\t': OS << "\\t"; break;
       case '\a': OS << "\\a"; break;
       case '\b': OS << "\\b"; break;
+      }
     }
   }
+  if (IsRaw)
+    OS << ")";
+  if (RawPrefix)
+    OS << RawPrefix;
   OS << '"';
 }
 
 void StringLiteral::setString(const ASTContext &C, StringRef Str,
-                              StringKind Kind, bool IsPascal) {
+                              StringKind Kind, bool IsPascal, bool IsRaw,
+                              StringRef RawPrefix) {
   //FIXME: we assume that the string data comes from a target that uses the same
   // code unit size and endianess for the type of string.
   this->Kind = Kind;
   this->IsPascal = IsPascal;
+  this->IsRaw = IsRaw;
+ 
+  if (!RawPrefix.empty()) {
+    assert(IsRaw && "Nonempty raw prefix for a string that is not raw?");
+    size_t PrefixLen = RawPrefix.size();
+    char *PrefixData = new (C) char[PrefixLen + 1];
+    std::memcpy(PrefixData, RawPrefix.data(), PrefixLen);
+    PrefixData[PrefixLen] = 0;
+    this->RawPrefix = PrefixData;
+  } else {
+    this->RawPrefix = nullptr;
+  }
   
   CharByteWidth = mapCharByteWidth(C.getTargetInfo(),Kind);
   assert((Str.size()%CharByteWidth == 0)

lib/Frontend/Rewrite/RewriteModernObjC.cpp

           Context->CharTy, llvm::APInt(32, Str.size() + 1), ArrayType::Normal,
           0);
       return StringLiteral::Create(*Context, Str, StringLiteral::Ascii,
-                                   /*Pascal=*/false, StrType, SourceLocation());
+                                   /*Pascal=*/false, /*Raw*/false, StringRef(),
+                                   StrType, SourceLocation());
     }
   };
   

lib/Frontend/Rewrite/RewriteObjC.cpp

           Context->CharTy, llvm::APInt(32, Str.size() + 1), ArrayType::Normal,
           0);
       return StringLiteral::Create(*Context, Str, StringLiteral::Ascii,
-                                   /*Pascal=*/false, StrType, SourceLocation());
+                                   /*Pascal=*/false, /*Raw*/false, StringRef(),
+                                   StrType, SourceLocation());
     }
   };
   

lib/Lex/LiteralSupport.cpp

   : SM(PP.getSourceManager()), Features(PP.getLangOpts()),
     Target(PP.getTargetInfo()), Diags(Complain ? &PP.getDiagnostics() :nullptr),
     MaxTokenLength(0), SizeBound(0), CharByteWidth(0), Kind(tok::unknown),
-    ResultPtr(ResultBuf.data()), hadError(false), Pascal(false) {
+    ResultPtr(ResultBuf.data()), hadError(false), Pascal(false), Raw(false) {
   init(StringToks);
 }
 

   ResultPtr = &ResultBuf[0];   // Next byte to fill in.
 
   Pascal = false;
+  Raw = false;
 
   SourceLocation UDSuffixTokLoc;
 

       const char *Prefix = ThisTokBuf;
       while (ThisTokBuf[0] != '(')
         ++ThisTokBuf;
+
+      // The raw prefix is everything between the R" and the (. However, we only
+      // track whether the literal is raw when all of the components of the
+      // literal are raw and use the same prefix.
+      StringRef PrefStr(Prefix, ThisTokBuf - Prefix);
+      if (0 == i || (Raw && !RawPrefix.compare(PrefStr))) {
+        Raw = true;
+        RawPrefix.assign(PrefStr);
+      } else {
+        Raw = false;
+        RawPrefix.clear();
+      }
+
       ++ThisTokBuf; // skip '('
 
       // Remove same number of characters from the end

lib/Sema/SemaExpr.cpp

 
   // Pass &StringTokLocs[0], StringTokLocs.size() to factory!
   StringLiteral *Lit = StringLiteral::Create(Context, Literal.GetString(),
-                                             Kind, Literal.Pascal, StrTy,
-                                             &StringTokLocs[0],
+                                             Kind, Literal.Pascal, Literal.Raw,
+                                             Literal.GetRawStringPrefix(),
+                                             StrTy, &StringTokLocs[0],
                                              StringTokLocs.size());
   if (Literal.getUDSuffix().empty())
     return Lit;

       ResTy = Context.getConstantArrayType(ResTy, LengthI, ArrayType::Normal,
                                            /*IndexTypeQuals*/ 0);
       SL = StringLiteral::Create(Context, RawChars, StringLiteral::Wide,
-                                 /*Pascal*/ false, ResTy, Loc);
+                                 /*Pascal*/ false, /*Raw*/ false, StringRef(),
+                                 ResTy, Loc);
     } else {
       ResTy = Context.CharTy.withConst();
       ResTy = Context.getConstantArrayType(ResTy, LengthI, ArrayType::Normal,
                                            /*IndexTypeQuals*/ 0);
       SL = StringLiteral::Create(Context, Str, StringLiteral::Ascii,
-                                 /*Pascal*/ false, ResTy, Loc);
+                                 /*Pascal*/ false, /*Raw*/ false, StringRef(),
+                                 ResTy, Loc);
     }
   }
 

           ArrayType::Normal, 0);
       Expr *Lit = StringLiteral::Create(
           Context, StringRef(TokSpelling.data(), Length), StringLiteral::Ascii,
-          /*Pascal*/false, StrTy, &TokLoc, 1);
+          /*Pascal*/false, /*Raw*/false, StringRef(), StrTy, &TokLoc, 1);
       return BuildLiteralOperatorCall(R, OpNameInfo, Lit, TokLoc);
     }
 

lib/Sema/SemaExprObjC.cpp

         CAT->getElementType(), llvm::APInt(32, StrBuf.size() + 1),
         CAT->getSizeModifier(), CAT->getIndexTypeCVRQualifiers());
     S = StringLiteral::Create(Context, StrBuf, StringLiteral::Ascii,
-                              /*Pascal=*/false, StrTy, &StrLocs[0],
-                              StrLocs.size());
+                              /*Pascal=*/false, /*Raw=*/false, StringRef(),
+                              StrTy, &StrLocs[0], StrLocs.size());
   }
-  
   return BuildObjCStringLiteral(AtLocs[0], S);
 }
 

lib/Serialization/ASTReaderStmt.cpp

   StringLiteral::StringKind kind =
         static_cast<StringLiteral::StringKind>(Record[Idx++]);
   bool isPascal = Record[Idx++];
+  bool isRaw = Record[Idx++];
+  unsigned RawPrefixLen = Record[Idx++];
+  SmallString<8> RawPrefix(&Record[Idx], &Record[Idx] + RawPrefixLen);
+  Idx += RawPrefixLen;
 
   // Read string data
   SmallString<16> Str(&Record[Idx], &Record[Idx] + Len);
-  E->setString(Reader.getContext(), Str, kind, isPascal);
+  E->setString(Reader.getContext(), Str, kind, isPascal, isRaw, RawPrefix);
   Idx += Len;
 
   // Read source locations

lib/Serialization/ASTWriterStmt.cpp

   Record.push_back(E->getNumConcatenated());
   Record.push_back(E->getKind());
   Record.push_back(E->isPascal());
+  Record.push_back(E->isRaw());
+  StringRef RawPrefix = E->getRawPrefix();
+  Record.push_back(RawPrefix.size());
+  Record.append(RawPrefix.begin(), RawPrefix.end());
+
   // FIXME: String data should be stored as a blob at the end of the
   // StringLiteral. However, we can't do so now because we have no
   // provision for coping with abbreviations when we're jumping around

test/Misc/ast-print-string-literal.cpp

+// RUN: %clang_cc1 -ast-print -std=c++1z %s -o - | FileCheck %s
+
+const char *S = R"T(This is a test)T";
+
+const wchar_t *WS = LR"Teehee(This is a test
+with a newline in it)Teehee";
+
+const char *T = u8R"(This is also a test)";
+
+const char *U = R"(This test has \b and a tab  )";
+
+const char *V = R"(This is a test )" R"test(that should concat to a non-raw string)test";
+
+const char *W = R"test(This is a test )test" R"test(that should concat to a raw string)test";
+
+const char *X = "This is a test " R"(that should concat to a non-raw string)";
+
+// CHECK: const char *S = R"T(This is a test)T";
+// CHECK: const wchar_t *WS = LR"Teehee(This is a test
+// CHECK-NEXT: with a newline in it)Teehee";
+// CHECK: const char *T = u8R"(This is also a test)";
+// CHECK: const char *U = R"(This test has \b and a tab  )";
+// CHECK: const char *V = "This is a test that should concat to a non-raw string";
+// CHECK: const char *W = R"test(This is a test that should concat to a raw string)test";
+// CHECK: const char *X = "This is a test that should concat to a non-raw string";

test/PCH/cxx-string-literal.cpp

+// RUN: %clang_cc1 -emit-pch -std=c++1z -o %t %s
+// RUN: %clang_cc1 -std=c++1z -x ast -ast-print %t | FileCheck %s
+
+const char *S = R"T(This is a test)T";
+// CHECK: const char *S = R"T(This is a test)T";
+
+const wchar_t *WS = LR"Teehee(This is a test
+with a newline in it)Teehee";
+// CHECK: const wchar_t *WS = LR"Teehee(This is a test
+// CHECK-NEXT: with a newline in it)Teehee";
+
+const char *T = u8R"(This is also a test)";
+// CHECK: const char *T = u8R"(This is also a test)";

test/SemaCXX/cxx11-ast-print.cpp

 
 // CHECK: const char *p1 = "bar1"_foo;
 const char *p1 = "bar1"_foo;
-// CHECK: const char *p2 = "bar2"_foo;
+// CHECK: const char *p2 = R"x(bar2)x"_foo;
 const char *p2 = R"x(bar2)x"_foo;
 // CHECK: const char *p3 = u8"bar3"_foo;
 const char *p3 = u8"bar3"_foo;