[flang] Rework preprocessing of stringification

[klausler]

Summary

Hew more closely to the C17 standard; perform macro replacement
of arguments to function-like macros unless they're being stringified
(if that's a word) or pasted. Test with a model "assert" macro idiom that
exposed the problem.

[PeteSteinfeld]

All builds, tests, and looks good.

[hubert.reinterpretcast]

Is this a documented assert idiom? Wouldn't macro expansion occur if STR(x) did not immediately apply the stringification (but instead expanded to an invocation of another macro)?

[klausler]

Fix was wrong -- it masked the actual problem rather than solving it. Here's a second take.

[PeteSteinfeld]

Things still build, test, and look good.

flang/lib/Parser/parsing.cpp

for (const auto &predef : options.predefinitions) {
     if (predef.second) {
       preprocessor.Define(predef.first, *predef.second);
     } else {
       preprocessor.Undefine(predef.first);
     }
   }
   currentCooked_ = &allCooked_.NewCookedSource();
   Prescanner prescanner{
-      messages_, *currentCooked_, allSources, preprocessor, options.features};
+      messages_, *currentCooked_, preprocessor, options.features};
   prescanner.set_fixedForm(options.isFixedForm)
       .set_fixedFormColumnLimit(options.fixedFormColumns)
       .AddCompilerDirectiveSentinel("dir$");
   if (options.features.IsEnabled(LanguageFeature::OpenACC)) {
     prescanner.AddCompilerDirectiveSentinel("$acc");
   }
   if (options.features.IsEnabled(LanguageFeature::OpenMP)) {
     prescanner.AddCompilerDirectiveSentinel("$omp");

flang/lib/Parser/preprocessor.h

 #include <stack>
 #include <string>
 #include <unordered_map>
 #include <vector>
 
 namespace Fortran::parser {
 
 class Prescanner;
+class Preprocessor;
 
 // Defines a macro
 class Definition {
 public:
   Definition(const TokenSequence &, std::size_t firstToken, std::size_t tokens);
   Definition(const std::vector<std::string> &argNames, const TokenSequence &,
       std::size_t firstToken, std::size_t tokens, bool isVariadic = false);
   Definition(const std::string &predefined, AllSources &);
 
   bool isFunctionLike() const { return isFunctionLike_; }
   std::size_t argumentCount() const { return argumentCount_; }
   bool isVariadic() const { return isVariadic_; }
   bool isDisabled() const { return isDisabled_; }
   bool isPredefined() const { return isPredefined_; }
   const TokenSequence &replacement() const { return replacement_; }
 
   bool set_isDisabled(bool disable);
 
-  TokenSequence Apply(const std::vector<TokenSequence> &args, AllSources &);
+  TokenSequence Apply(const std::vector<TokenSequence> &args, Prescanner &);
 
 private:
   static TokenSequence Tokenize(const std::vector<std::string> &argNames,
       const TokenSequence &token, std::size_t firstToken, std::size_t tokens);
 
   bool isFunctionLike_{false};
   std::size_t argumentCount_{0};
   bool isVariadic_{false};
   bool isDisabled_{false};
   bool isPredefined_{false};
   TokenSequence replacement_;
 };
 
 // Preprocessing state
 class Preprocessor {
 public:
   explicit Preprocessor(AllSources &);
 
+  const AllSources &allSources() const { return allSources_; }
+  AllSources &allSources() { return allSources_; }
+
   void Define(std::string macro, std::string value);
   void Undefine(std::string macro);
   bool IsNameDefined(const CharBlock &);
 
   std::optional<TokenSequence> MacroReplacement(
-      const TokenSequence &, const Prescanner &);
+      const TokenSequence &, Prescanner &);
 
   // Implements a preprocessor directive.
   void Directive(const TokenSequence &, Prescanner *);
 
 private:
   enum class IsElseActive { No, Yes };
   enum class CanDeadElseAppear { No, Yes };
 
   CharBlock SaveTokenAsName(const CharBlock &);
-  TokenSequence ReplaceMacros(const TokenSequence &, const Prescanner &);
+  TokenSequence ReplaceMacros(const TokenSequence &, Prescanner &);
   void SkipDisabledConditionalCode(
       const std::string &, IsElseActive, Prescanner *, ProvenanceRange);
   bool IsIfPredicateTrue(const TokenSequence &expr, std::size_t first,
       std::size_t exprTokens, Prescanner *);
 
   AllSources &allSources_;
   std::list<std::string> names_;
   std::unordered_map<CharBlock, Definition> definitions_;
   std::stack<CanDeadElseAppear> ifStack_;
 };
 } // namespace Fortran::parser
 #endif // FORTRAN_PARSER_PREPROCESSOR_H_

flang/lib/Parser/preprocessor.cpp

if (IsLegalIdentifierStart(tok)) {
         continue;
       }
     }
     result.Put(token, firstToken + j, 1);
   }
   return result;
 }
 
-static std::size_t AfterLastNonBlank(const TokenSequence &tokens) {
-  for (std::size_t j{tokens.SizeInTokens()}; j > 0; --j) {
-    if (!tokens.TokenAt(j - 1).IsBlank()) {
-      return j;
-    }
-  }
-  return 0;
-}
-
 static TokenSequence Stringify(
     const TokenSequence &tokens, AllSources &allSources) {
   TokenSequence result;
   Provenance quoteProvenance{allSources.CompilerInsertionProvenance('"')};
   result.PutNextTokenChar('"', quoteProvenance);
   for (std::size_t j{0}; j < tokens.SizeInTokens(); ++j) {
     const CharBlock &token{tokens.TokenAt(j)};
     std::size_t bytes{token.size()};
     for (std::size_t k{0}; k < bytes; ++k) {
       char ch{token[k]};
       Provenance from{tokens.GetTokenProvenance(j, k)};
       if (ch == '"' || ch == '\\') {
         result.PutNextTokenChar(ch, from);
       }
       result.PutNextTokenChar(ch, from);
     }
   }
   result.PutNextTokenChar('"', quoteProvenance);
   result.CloseToken();
   return result;
 }
 
-TokenSequence Definition::Apply(
-    const std::vector<TokenSequence> &args, AllSources &allSources) {
+constexpr bool IsTokenPasting(CharBlock opr) {
+  return opr.size() == 2 && opr[0] == '#' && opr[1] == '#';
+}
+
+static bool AnyTokenPasting(const TokenSequence &text) {
+  std::size_t tokens{text.SizeInTokens()};
+  for (std::size_t j{0}; j < tokens; ++j) {
+    if (IsTokenPasting(text.TokenAt(j))) {
+      return true;
+    }
+  }
+  return false;
+}
+
+static TokenSequence TokenPasting(TokenSequence &&text) {
+  if (!AnyTokenPasting(text)) {
+    return std::move(text);
+  }
   TokenSequence result;
+  std::size_t tokens{text.SizeInTokens()};
   bool pasting{false};
+  for (std::size_t j{0}; j < tokens; ++j) {
+    if (IsTokenPasting(text.TokenAt(j))) {
+      if (!pasting) {
+        while (!result.empty() &&
+            result.TokenAt(result.SizeInTokens() - 1).IsBlank()) {
+          result.pop_back();
+        }
+        if (!result.empty()) {
+          result.ReopenLastToken();
+          pasting = true;
+        }
+      }
+    } else if (pasting && text.TokenAt(j).IsBlank()) {
+    } else {
+      result.Put(text, j, 1);
+      pasting = false;
+    }
+  }
+  return result;
+}
+
+TokenSequence Definition::Apply(
+    const std::vector<TokenSequence> &args, Prescanner &prescanner) {
+  TokenSequence result;
   bool skipping{false};
   int parenthesesNesting{0};
   std::size_t tokens{replacement_.SizeInTokens()};
   for (std::size_t j{0}; j < tokens; ++j) {
-    const CharBlock &token{replacement_.TokenAt(j)};
+    CharBlock token{replacement_.TokenAt(j)};
     std::size_t bytes{token.size()};
     if (skipping) {
       if (bytes == 1) {
         if (token[0] == '(') {
           ++parenthesesNesting;
         } else if (token[0] == ')') {
           skipping = --parenthesesNesting > 0;
         }
       }
       continue;
     }
-    if (bytes == 2 && token[0] == '~') {
+    if (bytes == 2 && token[0] == '~') { // argument substitution
       std::size_t index = token[1] - 'A';
       if (index >= args.size()) {
         continue;
       }
-      std::size_t afterLastNonBlank{AfterLastNonBlank(result)};
-      if (afterLastNonBlank > 0 &&
-          result.TokenAt(afterLastNonBlank - 1).ToString() == "#") {
-        // stringifying
-        while (result.SizeInTokens() >= afterLastNonBlank) {
+      std::size_t prev{j};
+      while (prev > 0 && replacement_.TokenAt(prev - 1).IsBlank()) {
+        --prev;
+      }
+      if (prev > 0 && replacement_.TokenAt(prev - 1).size() == 1 &&
+          replacement_.TokenAt(prev - 1)[0] ==
+              '#') { // stringify argument without macro replacement
+        std::size_t resultSize{result.SizeInTokens()};
+        while (resultSize > 0 && result.TokenAt(resultSize - 1).empty()) {
           result.pop_back();
         }
-        result.Put(Stringify(args[index], allSources));
+        CHECK(resultSize > 0 &&
+            result.TokenAt(resultSize - 1) == replacement_.TokenAt(prev - 1));
+        result.pop_back();
+        result.Put(Stringify(args[index], prescanner.allSources()));
       } else {
-        std::size_t argTokens{args[index].SizeInTokens()};
-        for (std::size_t k{0}; k < argTokens; ++k) {
-          if (!pasting || !args[index].TokenAt(k).IsBlank()) {
-            result.Put(args[index], k);
-            pasting = false;
+        const TokenSequence *arg{&args[index]};
+        std::optional<TokenSequence> replaced;
+        // Don't replace macros in the actual argument if it is preceded or
+        // followed by the token-pasting operator ## in the replacement text.
+        if (prev == 0 || !IsTokenPasting(replacement_.TokenAt(prev - 1))) {
+          auto next{replacement_.SkipBlanks(j + 1)};
+          if (next >= tokens || !IsTokenPasting(replacement_.TokenAt(next))) {
+            // Apply macro replacement to the actual argument
+            replaced =
+                prescanner.preprocessor().MacroReplacement(*arg, prescanner);
+            if (replaced) {
+              arg = &*replaced;
             }
           }
         }
-    } else if (bytes == 2 && token[0] == '#' && token[1] == '#') {
-      // Token pasting operator in body (not expanded argument); discard any
-      // immediately preceding white space, then reopen the last token.
-      while (!result.empty() &&
-          result.TokenAt(result.SizeInTokens() - 1).IsBlank()) {
-        result.pop_back();
+        result.Put(DEREF(arg));
       }
-      if (!result.empty()) {
-        result.ReopenLastToken();
-        pasting = true;
-      }
-    } else if (pasting && token.IsBlank()) {
-      // Delete whitespace immediately following ## in the body.
     } else if (bytes == 11 && isVariadic_ &&
         token.ToString() == "__VA_ARGS__") {
-      Provenance commaProvenance{allSources.CompilerInsertionProvenance(',')};
+      Provenance commaProvenance{
+          prescanner.preprocessor().allSources().CompilerInsertionProvenance(
+              ',')};
       for (std::size_t k{argumentCount_}; k < args.size(); ++k) {
         if (k > argumentCount_) {
           result.Put(","s, commaProvenance);
         }
         result.Put(args[k]);
       }
     } else if (bytes == 10 && isVariadic_ && token.ToString() == "__VA_OPT__" &&
         j + 2 < tokens && replacement_.TokenAt(j + 1).ToString() == "(" &&
         parenthesesNesting == 0) {
       parenthesesNesting = 1;
       skipping = args.size() == argumentCount_;
       ++j;
     } else {
       if (bytes == 1 && parenthesesNesting > 0 && token[0] == '(') {
         ++parenthesesNesting;
       } else if (bytes == 1 && parenthesesNesting > 0 && token[0] == ')') {
         if (--parenthesesNesting == 0) {
           skipping = false;
           continue;
         }
       }
       result.Put(replacement_, j);
     }
   }
-  return result;
+  return TokenPasting(std::move(result));
 }
 
 static std::string FormatTime(const std::time_t &now, const char *format) {
   char buffer[16];
   return {buffer,
       std::strftime(buffer, sizeof buffer, format, std::localtime(&now))};
 }
 

 
 void Preprocessor::Define(std::string macro, std::string value) {
   definitions_.emplace(SaveTokenAsName(macro), Definition{value, allSources_});
 }
 
 void Preprocessor::Undefine(std::string macro) { definitions_.erase(macro); }
 
 std::optional<TokenSequence> Preprocessor::MacroReplacement(
-    const TokenSequence &input, const Prescanner &prescanner) {
+    const TokenSequence &input, Prescanner &prescanner) {
   // Do quick scan for any use of a defined name.
   std::size_t tokens{input.SizeInTokens()};
   std::size_t j;
   for (j = 0; j < tokens; ++j) {
     CharBlock token{input.TokenAt(j)};
     if (!token.empty() && IsLegalIdentifierStart(token[0]) &&
         IsNameDefined(token)) {
       break;
if (!def.isFunctionLike()) {
           ProvenanceRange insert{allSources_.AddCompilerInsertion(repl)};
           ProvenanceRange call{allSources_.AddMacroCall(
               insert, input.GetTokenProvenanceRange(j), repl)};
           result.Put(repl, call.start());
           continue;
         }
       }
       def.set_isDisabled(true);
-      TokenSequence replaced{ReplaceMacros(def.replacement(), prescanner)};
+      TokenSequence replaced{
+          TokenPasting(ReplaceMacros(def.replacement(), prescanner))};
       def.set_isDisabled(false);
       if (!replaced.empty()) {
         ProvenanceRange from{def.replacement().GetProvenanceRange()};
         ProvenanceRange use{input.GetTokenProvenanceRange(j)};
         ProvenanceRange newRange{
             allSources_.AddMacroCall(from, use, replaced.ToString())};
         result.Put(replaced, newRange);
       }
for (; j < tokens; ++j) {
     for (std::size_t n{0}; n < argStart.size(); ++n) {
       std::size_t at{argStart[n]};
       std::size_t count{
           (n + 1 == argStart.size() ? k : argStart[n + 1] - 1) - at};
       args.emplace_back(TokenSequence(input, at, count));
     }
     def.set_isDisabled(true);
     TokenSequence replaced{
-        ReplaceMacros(def.Apply(args, allSources_), prescanner)};
+        ReplaceMacros(def.Apply(args, prescanner), prescanner)};
     def.set_isDisabled(false);
     if (!replaced.empty()) {
       ProvenanceRange from{def.replacement().GetProvenanceRange()};
       ProvenanceRange use{input.GetIntervalProvenanceRange(j, k - j)};
       ProvenanceRange newRange{
           allSources_.AddMacroCall(from, use, replaced.ToString())};
       result.Put(replaced, newRange);
     }
     j = k; // advance to the terminal ')'
   }
   return result;
 }
 
 TokenSequence Preprocessor::ReplaceMacros(
-    const TokenSequence &tokens, const Prescanner &prescanner) {
+    const TokenSequence &tokens, Prescanner &prescanner) {
   if (std::optional<TokenSequence> repl{MacroReplacement(tokens, prescanner)}) {
     return std::move(*repl);
   }
   return tokens;
 }
 
 void Preprocessor::Directive(const TokenSequence &dir, Prescanner *prescanner) {
   std::size_t tokens{dir.SizeInTokens()};

flang/lib/Parser/prescan.h

 
 namespace Fortran::parser {
 
 class Messages;
 class Preprocessor;
 
 class Prescanner {
 public:
-  Prescanner(Messages &, CookedSource &, AllSources &, Preprocessor &,
+  Prescanner(Messages &, CookedSource &, Preprocessor &,
       common::LanguageFeatureControl);
   Prescanner(const Prescanner &);
 
-  Messages &messages() const { return messages_; }
+  const AllSources &allSources() const { return allSources_; }
+  AllSources &allSources() { return allSources_; }
+  const Messages &messages() const { return messages_; }
+  Messages &messages() { return messages_; }
+  const Preprocessor &preprocessor() const { return preprocessor_; }
+  Preprocessor &preprocessor() { return preprocessor_; }
 
   Prescanner &set_fixedForm(bool yes) {
     inFixedForm_ = yes;
     return *this;
   }
   Prescanner &set_encoding(Encoding code) {
     encoding_ = code;
     return *this;
private:
   std::optional<LineClassification> IsFreeFormCompilerDirectiveLine(
       const char *) const;
   const char *IsCompilerDirectiveSentinel(const char *) const;
   LineClassification ClassifyLine(const char *) const;
   void SourceFormChange(std::string &&);
 
   Messages &messages_;
   CookedSource &cooked_;
-  AllSources &allSources_;
   Preprocessor &preprocessor_;
+  AllSources &allSources_;
   common::LanguageFeatureControl features_;
   bool inFixedForm_{false};
   int fixedFormColumnLimit_{72};
   Encoding encoding_{Encoding::UTF_8};
   int delimiterNesting_{0};
   int prescannerNesting_{0};
 
   Provenance startProvenance_;

flang/lib/Parser/prescan.cpp

 
 namespace Fortran::parser {
 
 using common::LanguageFeature;
 
 static constexpr int maxPrescannerNesting{100};
 
 Prescanner::Prescanner(Messages &messages, CookedSource &cooked,
-    AllSources &allSources, Preprocessor &preprocessor,
-    common::LanguageFeatureControl lfc)
-    : messages_{messages}, cooked_{cooked}, allSources_{allSources},
-      preprocessor_{preprocessor}, features_{lfc},
+    Preprocessor &preprocessor, common::LanguageFeatureControl lfc)
+    : messages_{messages}, cooked_{cooked}, preprocessor_{preprocessor},
+      allSources_{preprocessor_.allSources()}, features_{lfc},
       encoding_{allSources_.encoding()} {}
 
 Prescanner::Prescanner(const Prescanner &that)
     : messages_{that.messages_}, cooked_{that.cooked_},
-      allSources_{that.allSources_}, preprocessor_{that.preprocessor_},
+      preprocessor_{that.preprocessor_}, allSources_{that.allSources_},
       features_{that.features_}, inFixedForm_{that.inFixedForm_},
       fixedFormColumnLimit_{that.fixedFormColumnLimit_},
       encoding_{that.encoding_}, prescannerNesting_{that.prescannerNesting_ +
                                      1},
       skipLeadingAmpersand_{that.skipLeadingAmpersand_},
       compilerDirectiveBloomFilter_{that.compilerDirectiveBloomFilter_},
       compilerDirectiveSentinels_{that.compilerDirectiveSentinels_} {}
 
if ((*at_ == 'h' || *at_ == 'H') && n > 0 && n < maxHollerith &&
         inPreprocessorDirective_) {
       do {
         EmitCharAndAdvance(tokens, *at_);
       } while (IsHexadecimalDigit(*at_));
     } else if (IsLetter(*at_)) {
       // Handles FORMAT(3I9HHOLLERITH) by skipping over the first I so that
       // we don't misrecognize I9HOLLERITH as an identifier in the next case.
       EmitCharAndAdvance(tokens, *at_);
-    } else if (at_[0] == '_' && (at_[1] == '\'' || at_[1] == '"')) {
+    } else if (at_[0] == '_' && (at_[1] == '\'' || at_[1] == '"')) { // 4_"..."
       EmitCharAndAdvance(tokens, *at_);
       QuotedCharacterLiteral(tokens, start);
     }
     preventHollerith_ = false;
   } else if (*at_ == '.') {
     char nch{EmitCharAndAdvance(tokens, '.')};
     if (!inPreprocessorDirective_ && IsDecimalDigit(nch)) {
       while (IsDecimalDigit(EmitCharAndAdvance(tokens, *at_))) {
       }
       ExponentAndKind(tokens);
     } else if (nch == '.' && EmitCharAndAdvance(tokens, '.') == '.') {
       EmitCharAndAdvance(tokens, '.'); // variadic macro definition ellipsis
     }
     preventHollerith_ = false;
   } else if (IsLegalInIdentifier(*at_)) {
     do {
     } while (IsLegalInIdentifier(EmitCharAndAdvance(tokens, *at_)));
-    if (*at_ == '\'' || *at_ == '"') {
+    if ((*at_ == '\'' || *at_ == '"') &&
+        tokens.CharAt(tokens.SizeInChars() - 1) == '_') { // kind_"..."
       QuotedCharacterLiteral(tokens, start);
     }
     preventHollerith_ = false;
   } else if (*at_ == '*') {
     if (EmitCharAndAdvance(tokens, '*') == '*') {
       EmitCharAndAdvance(tokens, '*');
     } else {
       // Subtle ambiguity:

flang/test/Preprocessing/assert.F90

+!RUN: %f18 -E %s 2>&1 | FileCheck %s
+!CHECK: if(.not.(.true.)) error stop "assert(" // ".TRUE." // ") failed " // "at ""
+!CHECK-SAME: assert.F90"": " // "7"
+#define STR(x) #x
+#define POSITION(f,ln) "at "f": " // STR(ln)
+#define assert(x) if(.not.(x)) error stop "assert(" // #x // ") failed " // POSITION(__FILE__,__LINE__)
+assert(.TRUE.)
+end