Address naming/documentation-level review feedback

Author: apple-fcloutier

clang/docs/ReleaseNotes.rst

@@ -382,8 +382,39 @@ related warnings within the method body.
   ``format_matches`` accepts an example valid format string as its third
   argument. For more information, see the Clang attributes documentation.
 
-- Format string checking now supports the compile-time evaluation of format
-  strings as a fallback mechanism.
+- Clang can now verify format strings that can be constant-folded even if they
+  do not resolve to a string literal. For instance, all of these can now be
+  verified:
+
+  .. code-block:: c++
+
+    const char format[] = {'h', 'e', 'l', 'l', 'o', ' ', '%', 's', 0};
+    printf(format, "world");
+    // no warning
+    
+    printf(format, 123);
+    // warning: format specifies type 'char *' but the argument has type 'int'
+
+    printf(("%"s + "i"s).c_str(), "world");
+    // warning: format specifies type 'int' but the argument has type 'char *'
+  
+  When the format expression does not evaluate to a string literal, Clang
+  points diagnostics into a pseudo-file called ``<scratch space>`` that contains
+  the format string literal as it evaluated, like so:
+
+  .. code-block:: text
+
+    example.c:6:17: warning: format specifies type 'char *' but the argument has type 'int' [-Wformat]
+        6 |     printf(format, 123);
+          |            ~~~~~~  ^~~
+    <scratch space>:1:4: note: format string resolved to a constant string
+        1 | "hello %s"
+          |        ^~
+          |        %d
+  
+  This may mean that format strings which were previously unverified (or which
+  triggered ``-Wformat-nonliteral``) are now verified by ``-Wformat`` and its
+  allies.
 
 - Introduced a new statement attribute ``[[clang::atomic]]`` that enables
   fine-grained control over atomic code generation on a per-statement basis.

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -10355,7 +10355,7 @@ def warn_format_bool_as_character : Warning<
   InGroup<Format>;
 def note_format_string_defined : Note<"format string is defined here">;
 def note_format_string_evaluated_to : Note<
-  "format string was constant-evaluated">;
+  "format string resolved to a constant string">;
 def note_format_fix_specifier : Note<"did you mean to use '%0'?">;
 def note_printf_c_str: Note<"did you mean to call the %0 method?">;
 def note_format_security_fixit: Note<

clang/lib/AST/ExprConstant.cpp

@@ -17981,14 +17981,17 @@ static bool EvaluateStringAsLValue(EvalInfo &Info, const Expr *E,
       String.addArray(Info, E, CAT);
     else
       String.addUnsizedArray(Info, E, CharTy);
-  } else if (Ty->hasPointerRepresentation()) {
+    return true;
+  }
+
+  if (Ty->hasPointerRepresentation()) {
     if (!EvaluatePointer(E, String, Info))
       return false;
     CharTy = Ty->getPointeeType();
-  } else {
-    return false;
+    return true;
   }
-  return true;
+
+  return false;
 }
 
 static const StringLiteral *StringLValueIsLiteral(EvalInfo &Info,

clang/lib/Sema/SemaChecking.cpp

@@ -6050,15 +6050,22 @@ static void CheckFormatString(
     llvm::SmallBitVector &CheckedVarArgs, UncoveredArgHandler &UncoveredArg,
     bool IgnoreStringsWithoutSpecifiers);
 
-enum StringLiteralConstEvalResult {
+enum StringLiteralConstantEvaluationResult {
   SLCER_NotEvaluated,
   SLCER_NotNullTerminated,
   SLCER_Evaluated,
 };
 
-static StringLiteralConstEvalResult
-constEvalStringAsLiteral(Sema &S, const Expr *E, const StringLiteral *&SL,
-                         uint64_t &Offset);
+/// Attempt to fold \c E into a constant string that \c checkFormatStringExpr
+/// can use. If \c E folds to a string literal, that string literal will be used
+/// for diagnostics. If \c E has a constant string value but it does not fold to
+/// a literal (for instance, ("%"s + "i"s).c_str() constant-folds to "%i"), a
+/// <scratch space> pseudo-source file will be allocated, containing a string
+/// literal representation of the constant string, and format diagnostics will
+/// point to it.
+static StringLiteralConstantEvaluationResult
+EvaluateStringAndCreateLiteral(Sema &S, const Expr *E, const StringLiteral *&SL,
+                               uint64_t &Offset);
 
 // Determine if an expression is a string literal or constant string.
 // If this function returns false on the arguments to a function expecting a
@@ -6447,7 +6454,7 @@ static StringLiteralCheckType checkFormatStringExpr(
 
   uint64_t EvalOffset = 0;
   const StringLiteral *FakeLiteral = nullptr;
-  switch (constEvalStringAsLiteral(S, E, FakeLiteral, EvalOffset)) {
+  switch (EvaluateStringAndCreateLiteral(S, E, FakeLiteral, EvalOffset)) {
   case SLCER_NotEvaluated:
     return SLCT_NotALiteral;
 
@@ -6469,9 +6476,9 @@ static StringLiteralCheckType checkFormatStringExpr(
   }
 }
 
-static StringLiteralConstEvalResult
-constEvalStringAsLiteral(Sema &S, const Expr *E, const StringLiteral *&SL,
-                         uint64_t &Offset) {
+static StringLiteralConstantEvaluationResult
+EvaluateStringAndCreateLiteral(Sema &S, const Expr *E, const StringLiteral *&SL,
+                               uint64_t &Offset) {
   // As a last resort, try to constant-evaluate the format string.
   bool HasNul;
   auto SER = E->tryEvaluateString(S.Context, &HasNul);
@@ -6500,7 +6507,7 @@ constEvalStringAsLiteral(Sema &S, const Expr *E, const StringLiteral *&SL,
                                                    "<scratch space>", true));
   }
 
-  auto ScratchFile = S.getSourceManager().createFileID(std::move(MemBuf));
+  FileID ScratchFile = S.getSourceManager().createFileID(std::move(MemBuf));
   SourceLocation Begin = S.getSourceManager().getLocForStartOfFile(ScratchFile);
   QualType SLType = S.Context.getStringLiteralArrayType(
       S.Context.CharTy, SER->getString().size());