[ms compatibility] Fix for PR9984.

[ABataev]

Summary

Fixed incompatibility in lexer with MSVC for a wide string literal from L#macro_arg in a function-like macro

[majnemer]

include/clang/Lex/Token.h

69 ↗

(On Diff #17147)

Can we make this unsigned short instead?

268 ↗

(On Diff #17147)

is formed by macro

lib/Lex/TokenLexer.cpp

441 ↗

(On Diff #17147)

processing

442 ↗

(On Diff #17147)

References to bug reports shouldn't be in the source code. Instead, I'd have a description of what the special logic is trying to accomplish.

443–445 ↗

(On Diff #17147)

This if is getting pretty gnarly, maybe it would be a little cleaner if you broke this logic up a little bit.

[ABataev]

David, thanks for the review!

include/clang/Lex/Token.h

69 ↗

(On Diff #17147)

Yes, of course, I'll do it

268 ↗

(On Diff #17147)

Will be fixed, thanks.

lib/Lex/TokenLexer.cpp

441 ↗

(On Diff #17147)

Thanks

442 ↗

(On Diff #17147)

Ok, will be fixed

443–445 ↗

(On Diff #17147)

Ok, I'll split it

[majnemer]

Can we get rid of our kw_L__FUNCTION__ hack with this change?

lib/Lex/TokenLexer.cpp

524 ↗

(On Diff #17147)

What's the rationale behind this change?

[ABataev]

Update after review

[ABataev]

lib/Lex/TokenLexer.cpp

524 ↗

(On Diff #17147)

In construct L#x we don't have ## that should be skipped. So I just check it and skip this ## only if it exists.

[ABataev]

Can we get rid of our kw_L__FUNCTION__ hack with this change?

No, I don't think we can. My solution fixes only lexer, but when we building LFUNCTION we're building an expression.

Best regards,

Alexey Bataev

Software Engineer
Intel Compiler Team

12.12.2014 7:39, David Majnemer пишет:

Can we get rid of our kw_L__FUNCTION__ hack with this change?

Comment at: lib/Lex/TokenLexer.cpp:524
@@ -515,2 +523,3 @@

PasteOpLoc = Tokens[CurToken].getLocation();

• ++CurToken;

+ if (Tokens[CurToken].is(tok::hashhash))

+ ++CurToken;

What's the rationale behind this change?

http://reviews.llvm.org/D6604

EMAIL PREFERENCES

http://reviews.llvm.org/settings/panel/emailpreferences/

[rnk]

lgtm

include/clang/Lex/Token.h

69 ↗

(On Diff #17214)

OK, I guess either way there is no overall size increase. We could probably pack Kind and Flags more tightly, but that's separable.

lib/Lex/TokenLexer.cpp

410 ↗

(On Diff #17214)

The convention for functions uses a leading lower case.

448–454 ↗

(On Diff #17214)

Let's rearrange this to check the Macro bool first so we can short-circuit out with the least amount of work.

[ABataev]

Reid, thanks for the review!

lib/Lex/TokenLexer.cpp

410 ↗

(On Diff #17214)

Ok, fixed.

448–454 ↗

(On Diff #17214)

Agree, fixed

cfe/trunk/include/clang/Lex/Token.h

class Token {
   ///  Other:
   ///    This is null.
   void *PtrData;
 
   /// Kind - The actual flavor of token this is.
   tok::TokenKind Kind;
 
   /// Flags - Bits we track about this token, members of the TokenFlags enum.
-  unsigned char Flags;
+  unsigned short Flags;
 public:
 
   // Various flags set per token:
   enum TokenFlags {
     StartOfLine   = 0x01,  // At start of line or only after whitespace
                            // (considering the line after macro expansion).
     LeadingSpace  = 0x02,  // Whitespace exists before this token (considering 
                            // whitespace after macro expansion).
     DisableExpand = 0x04,  // This identifier may never be macro expanded.
     NeedsCleaning = 0x08,  // Contained an escaped newline or trigraph.
     LeadingEmptyMacro = 0x10, // Empty macro exists before this token.
     HasUDSuffix = 0x20,    // This string or character literal has a ud-suffix.
     HasUCN = 0x40,         // This identifier contains a UCN.
-    IgnoredComma = 0x80    // This comma is not a macro argument separator (MS).
+    IgnoredComma = 0x80,   // This comma is not a macro argument separator (MS).
+    StringifiedInMacro = 0x100, // This string or character literal is formed by
+                                // macro stringizing or charizing operator.
   };
 
   tok::TokenKind getKind() const { return Kind; }
   void setKind(tok::TokenKind K) { Kind = K; }
 
   /// is/isNot - Predicates to check if this token is a specific kind, as in
   /// "if (Tok.is(tok::l_brace)) {...}".
   bool is(tok::TokenKind K) const { return Kind == K; }
public:
   }
 
   /// \brief Return true if this token is a string or character literal which
   /// has a ud-suffix.
   bool hasUDSuffix() const { return (Flags & HasUDSuffix) ? true : false; }
 
   /// Returns true if this token contains a universal character name.
   bool hasUCN() const { return (Flags & HasUCN) ? true : false; }
+
+  /// Returns true if this token is formed by macro by stringizing or charizing
+  /// operator.
+  bool stringifiedInMacro() const {
+    return (Flags & StringifiedInMacro) ? true : false;
+  }
 };
 
 /// \brief Information about the conditional stack (\#if directives)
 /// currently active.
 struct PPConditionalInfo {
   /// \brief Location where the conditional started.
   SourceLocation IfLoc;
 

cfe/trunk/lib/Lex/TokenLexer.cpp

if (CurTok.is(tok::hash) || CurTok.is(tok::hashat)) {
                                                  ExpansionLocEnd);
       else {
         // 'charify': don't bother caching these.
         Res = MacroArgs::StringifyArgument(ActualArgs->getUnexpArgument(ArgNo),
                                            PP, true,
                                            ExpansionLocStart,
                                            ExpansionLocEnd);
       }
+      Res.setFlag(Token::StringifiedInMacro);
 
       // The stringified/charified string leading space flag gets set to match
       // the #/#@ operator.
       if (NextTokGetsSpace)
         Res.setFlag(Token::LeadingSpace);
 
       ResultToks.push_back(Res);
       MadeChange = true;
if (MadeChange) {
     // when this TokenLexer finishes lexing them.
     Tokens = PP.cacheMacroExpandedTokens(this, ResultToks);
 
     // The preprocessor cache of macro expanded tokens owns these tokens,not us.
     OwnsTokens = false;
   }
 }
 
+/// \brief Checks if two tokens form wide string literal.
+static bool isWideStringLiteralFromMacro(const Token &FirstTok,
+                                         const Token &SecondTok) {
+  return FirstTok.is(tok::identifier) &&
+         FirstTok.getIdentifierInfo()->isStr("L") && SecondTok.isLiteral() &&
+         SecondTok.stringifiedInMacro();
+}
+
 /// Lex - Lex and return a token from this macro stream.
 ///
 bool TokenLexer::Lex(Token &Tok) {
   // Lexing off the end of the macro, pop this macro off the expansion stack.
   if (isAtEnd()) {
     // If this is a macro (not a token stream), mark the macro enabled now
     // that it is no longer being expanded.
     if (Macro) Macro->EnableMacro();
bool TokenLexer::Lex(Token &Tok) {
 
   // Get the next token to return.
   Tok = Tokens[CurToken++];
 
   bool TokenIsFromPaste = false;
 
   // If this token is followed by a token paste (##) operator, paste the tokens!
   // Note that ## is a normal token when not expanding a macro.
-  if (!isAtEnd() && Tokens[CurToken].is(tok::hashhash) && Macro) {
+  if (!isAtEnd() && Macro &&
+      (Tokens[CurToken].is(tok::hashhash) ||
+       // Special processing of L#x macros in -fms-compatibility mode.
+       // Microsoft compiler is able to form a wide string literal from
+       // 'L#macro_arg' construct in a function-like macro.
+       (PP.getLangOpts().MSVCCompat &&
+        isWideStringLiteralFromMacro(Tok, Tokens[CurToken])))) {
     // When handling the microsoft /##/ extension, the final token is
     // returned by PasteTokens, not the pasted token.
     if (PasteTokens(Tok))
       return true;
 
     TokenIsFromPaste = true;
   }
 

 /// are more ## after it, chomp them iteratively.  Return the result as Tok.
 /// If this returns true, the caller should immediately return the token.
 bool TokenLexer::PasteTokens(Token &Tok) {
   SmallString<128> Buffer;
   const char *ResultTokStrPtr = nullptr;
   SourceLocation StartLoc = Tok.getLocation();
   SourceLocation PasteOpLoc;
   do {
-    // Consume the ## operator.
+    // Consume the ## operator if any.
     PasteOpLoc = Tokens[CurToken].getLocation();
+    if (Tokens[CurToken].is(tok::hashhash))
       ++CurToken;
     assert(!isAtEnd() && "No token on the RHS of a paste operator!");
 
     // Get the RHS token.
     const Token &RHS = Tokens[CurToken];
 
     // Allocate space for the result token.  This is guaranteed to be enough for
     // the two tokens.
     Buffer.resize(Tok.getLength() + RHS.getLength());

cfe/trunk/test/Lexer/ms-compatibility.c

+// RUN: %clang_cc1 -fsyntax-only -E -fms-compatibility %s | FileCheck --check-prefix=CHECK-MS-COMPAT %s
+// RUN: %clang_cc1 -fsyntax-only -E %s | FileCheck --check-prefix=CHECK-NO-MS-COMPAT %s
+
+#define FN(x) L#x
+#define F L "aaa"
+void *v1 = FN(aaa);
+void *v2 = F;
+// CHECK-MS-COMPAT: void *v1 = L"aaa";
+// CHECK-MS-COMPAT: void *v2 = L "aaa";
+// CHECK-NO-MS-COMPAT: void *v1 = L "aaa";
+// CHECK-NO-MS-COMPAT: void *v2 = L "aaa";