Merge pull request swiftlang#31129 from ravikandhadai/oslog-diagnostic-improvement

 [OSLog Diagnostics] Improve os log diagnostics emitted in the SIL and Sema diagnostic passes.

Author: ravikandhadai

include/swift/AST/DiagnosticsSIL.def

@@ -585,8 +585,8 @@ ERROR(oslog_constant_eval_trap, none, "%0", (StringRef))
 ERROR(oslog_too_many_instructions, none, "interpolated expression and arguments "
       "are too complex", ())
 
-ERROR(oslog_invalid_log_message, none, "invalid log message; do not define "
-      "extensions to types defined in the os module", ())
+ERROR(oslog_invalid_log_message, none, "invalid log message; extending "
+      "types defined in the os module is not supported", ())
 
 NOTE(oslog_const_evaluable_fun_error, none, "'%0' failed evaluation", (StringRef))
 
@@ -599,8 +599,9 @@ ERROR(oslog_non_constant_interpolation, none, "'OSLogInterpolation' instance "
 ERROR(oslog_property_not_constant, none, "'OSLogInterpolation.%0' is not a "
       "constant", (StringRef))
 
-ERROR(oslog_message_alive_after_opts, none, "os log string interpolation cannot "
-      "be used in this context", ())
+ERROR(oslog_message_alive_after_opts, none, "string interpolation cannot "
+      "be used in this context; if you are calling an os_log function, "
+      "try a different overload", ())
 
 ERROR(oslog_message_explicitly_created, none, "'OSLogMessage' must be "
       " created from a string interpolation or string literal", ())
@@ -609,7 +610,7 @@ WARNING(oslog_call_in_unreachable_code, none, "os log call will never be "
         "executed and may have undiagnosed errors", ())
 
 ERROR(global_string_pointer_on_non_constant, none, "globalStringTablePointer "
-      "builtin must used only on string literals", ())
+      "builtin must be used only on string literals", ())
 
 ERROR(polymorphic_builtin_passed_non_trivial_non_builtin_type, none, "Argument "
       "of type %0 can not be passed as an argument to a Polymorphic "

lib/SILOptimizer/Mandatory/OSLogOptimization.cpp

@@ -1210,7 +1210,9 @@ static void deleteInstructionWithUsersAndFixLifetimes(
 /// Try to dead-code eliminate the OSLogMessage instance \c oslogMessage passed
 /// to the os log call and clean up its dependencies. If the instance cannot be
 /// eliminated, emit diagnostics.
-static void tryEliminateOSLogMessage(SingleValueInstruction *oslogMessage) {
+/// \returns true if elimination is successful and false if it is not successful
+/// and diagnostics is emitted.
+static bool tryEliminateOSLogMessage(SingleValueInstruction *oslogMessage) {
   InstructionDeleter deleter;
   // List of instructions that are possibly dead.
   SmallVector<SILInstruction *, 4> worklist = {oslogMessage};
@@ -1249,13 +1251,15 @@ static void tryEliminateOSLogMessage(SingleValueInstruction *oslogMessage) {
     SILFunction *fun = oslogMessage->getFunction();
     diagnose(fun->getASTContext(), oslogMessage->getLoc().getSourceLoc(),
              diag::oslog_message_alive_after_opts);
+    return false;
   }
+  return true;
 }
 
 /// Constant evaluate instructions starting from \p start and fold the uses
 /// of the SIL value \p oslogMessage.
-/// \returns true if the body of the function containing \p oslogMessage is
-/// modified. Returns false otherwise.
+/// \returns true if folding is successful and false if it is not successful and
+/// diagnostics is emitted.
 static bool constantFold(SILInstruction *start,
                          SingleValueInstruction *oslogMessage,
                          unsigned assertConfig) {
@@ -1282,9 +1286,7 @@ static bool constantFold(SILInstruction *start,
     return false;
 
   substituteConstants(state);
-
-  tryEliminateOSLogMessage(oslogMessage);
-  return true;
+  return tryEliminateOSLogMessage(oslogMessage);
 }
 
 /// Given a call to the initializer of OSLogMessage, which conforms to
@@ -1548,14 +1550,14 @@ class OSLogOptimization : public SILFunctionTransform {
         // The log call is in unreachable code here.
         continue;
       }
-      bool bodyModified =
+      bool foldingSucceeded =
           constantFold(interpolationStart, oslogInit, assertConfig);
-      // If body was not modified, it implies that an error was diagnosed.
+      // If folding did not succeeded, it implies that an error was diagnosed.
       // However, this will also trigger a diagnostics later on since
       // _globalStringTablePointerBuiltin would not be passed a string literal.
       // Suppress this error by synthesizing a dummy string literal for the
       // builtin.
-      if (!bodyModified)
+      if (!foldingSucceeded)
         suppressGlobalStringTablePointerError(oslogInit);
       madeChange = true;
     }

lib/Sema/ConstantnessSemaDiagnostics.cpp

@@ -162,6 +162,12 @@ static Expr *checkConstantness(Expr *expr) {
     if (!isa<ApplyExpr>(expr))
       return expr;
 
+    if (NominalTypeDecl *nominal =
+        expr->getType()->getNominalOrBoundGenericNominal()) {
+      if (nominal->getName() == nominal->getASTContext().Id_OSLogMessage)
+        return expr;
+    }
+
     ApplyExpr *apply = cast<ApplyExpr>(expr);
     ValueDecl *calledValue = apply->getCalledValue();
     if (!calledValue)

test/SILOptimizer/OSLogCompilerDiagnosticsTest.swift

@@ -27,7 +27,7 @@ extension OSLogInterpolation {
 
 func testOSLogInterpolationExtension(a: A) {
   _osLogTestHelper("Error at line: \(a: a)")
-    // expected-error @-1 {{invalid log message; do not define extensions to types defined in the os module}}
+    // expected-error @-1 {{invalid log message; extending types defined in the os module is not supported}}
     // expected-note @-2 {{'OSLogInterpolation.appendLiteral(_:)' failed evaluation}}
     // expected-note @-3 {{value mutable by an unevaluated instruction is not a constant}}
 }
@@ -87,6 +87,6 @@ func testUnreachableLogCallComplex(c: Color)  {
   default: // expected-warning {{default will never be executed}}
     _osLogTestHelper("Some call \(c)")
       // expected-warning@-1 {{os log call will never be executed and may have undiagnosed errors}}
-      // expected-error@-2 {{globalStringTablePointer builtin must used only on string literals}}
+      // expected-error@-2 {{globalStringTablePointer builtin must be used only on string literals}}
   }
 }

test/SILOptimizer/diagnostic_constant_propagation.swift

@@ -361,7 +361,7 @@ func testBuiltinGlobalStringTablePointerNoError() -> UnsafePointer<CChar> {
 }
 
 func testBuiltinGlobalStringTablePointerError(s: String) -> UnsafePointer<CChar> {
-  return _getGlobalStringTablePointer(s) // expected-error {{globalStringTablePointer builtin must used only on string literals}}
+  return _getGlobalStringTablePointer(s) // expected-error {{globalStringTablePointer builtin must be used only on string literals}}
 }
 
 @_transparent

test/Sema/diag_constantness_check_os_log.swift

@@ -119,6 +119,17 @@ func testOSLogInterpolationExtension(a: MyStruct) {
   _osLogTestHelper("Error at line: \(a: a)")
 }
 
+func testExplicitOSLogMessageConstruction() {
+  _osLogTestHelper(OSLogMessage(stringLiteral: "world"))
+    // expected-error@-1 {{argument must be a string interpolation}}
+  _osLogTestHelper(
+    OSLogMessage( // expected-error {{argument must be a string interpolation}}
+      stringInterpolation:
+        OSLogInterpolation(
+          literalCapacity: 0,
+          interpolationCount: 0)))
+}
+
 // Test that @_semantics("oslog.log_with_level") permits values of type OSLog and
 // OSLogType to not be constants.