clang_generator: Turn fatal error on struc-by-value into warning

An `absl::StatusCode` of `kCancelled` now indicates warnings inside the emitter.

PiperOrigin-RevId: 485851898
Change-Id: I10a57cbc1b6c2d4b708c3c19aa0fa71451845a22

sandboxed_api/tools/clang_generator/diagnostics.cc

@@ -14,7 +14,9 @@
 
 #include "sandboxed_api/tools/clang_generator/diagnostics.h"
 
+#include "absl/status/status.h"
 #include "absl/strings/cord.h"
+#include "clang/Basic/Diagnostic.h"
 
 namespace sapi {
 
@@ -22,8 +24,9 @@ constexpr absl::string_view kSapiStatusPayload =
     "https://github.com/google/sandboxed-api";
 
 absl::Status MakeStatusWithDiagnostic(clang::SourceLocation loc,
+                                      absl::StatusCode code,
                                       absl::string_view message) {
-  absl::Status status = absl::UnknownError(message);
+  absl::Status status(code, message);
   absl::Cord payload;
   uint64_t raw_loc = loc.getRawEncoding();
   payload.Append(
@@ -32,6 +35,11 @@ absl::Status MakeStatusWithDiagnostic(clang::SourceLocation loc,
   return status;
 }
 
+absl::Status MakeStatusWithDiagnostic(clang::SourceLocation loc,
+                                      absl::string_view message) {
+  return MakeStatusWithDiagnostic(loc, absl::StatusCode::kUnknown, message);
+}
+
 absl::optional<clang::SourceLocation> GetDiagnosticLocationFromStatus(
     const absl::Status& status) {
   if (auto payload =
@@ -43,14 +51,31 @@ absl::optional<clang::SourceLocation> GetDiagnosticLocationFromStatus(
   return absl::nullopt;
 }
 
-clang::DiagnosticBuilder ReportFatalError(clang::DiagnosticsEngine& de,
+namespace {
-                                          clang::SourceLocation loc,
+
-                                          absl::string_view message) {
+clang::DiagnosticBuilder GetDiagnosticBuilder(
+    clang::DiagnosticsEngine& de, clang::SourceLocation loc,
+    clang::DiagnosticsEngine::Level level, absl::string_view message) {
   clang::DiagnosticBuilder builder =
-      de.Report(loc, de.getCustomDiagID(clang::DiagnosticsEngine::Fatal,
+      de.Report(loc, de.getCustomDiagID(level, "header generation: %0"));
-                                        "header generation failed: %0"));
   builder.AddString(llvm::StringRef(message.data(), message.size()));
   return builder;
 }
 
+}  // namespace
+
+clang::DiagnosticBuilder ReportFatalError(clang::DiagnosticsEngine& de,
+                                          clang::SourceLocation loc,
+                                          absl::string_view message) {
+  return GetDiagnosticBuilder(de, loc, clang::DiagnosticsEngine::Fatal,
+                              message);
+}
+
+clang::DiagnosticBuilder ReportWarning(clang::DiagnosticsEngine& de,
+                                       clang::SourceLocation loc,
+                                       absl::string_view message) {
+  return GetDiagnosticBuilder(de, loc, clang::DiagnosticsEngine::Warning,
+                              message);
+}
+
 }  // namespace sapi

sandboxed_api/tools/clang_generator/diagnostics.h

@@ -23,6 +23,12 @@
 
 namespace sapi {
 
+// Returns a new status with a payload that encodes the specified Clang source
+// location.
+absl::Status MakeStatusWithDiagnostic(clang::SourceLocation loc,
+                                      absl::StatusCode code,
+                                      absl::string_view message);
+
 // Returns a new UNKNOWN status with a payload that encodes the specified Clang
 // source location.
 absl::Status MakeStatusWithDiagnostic(clang::SourceLocation loc,
@@ -32,6 +38,10 @@ absl::Status MakeStatusWithDiagnostic(clang::SourceLocation loc,
 absl::optional<clang::SourceLocation> GetDiagnosticLocationFromStatus(
     const absl::Status& status);
 
+clang::DiagnosticBuilder ReportWarning(clang::DiagnosticsEngine& de,
+                                       clang::SourceLocation loc,
+                                       absl::string_view message);
+
 clang::DiagnosticBuilder ReportFatalError(clang::DiagnosticsEngine& de,
                                           clang::SourceLocation loc,
                                           absl::string_view message);

sandboxed_api/tools/clang_generator/emitter.cc

@@ -298,8 +298,9 @@ absl::StatusOr<std::string> EmitFunction(const clang::FunctionDecl* decl) {
   auto function_name = ToStringView(decl->getName());
   const clang::QualType return_type = decl->getDeclaredReturnType();
   if (return_type->isRecordType()) {
-    return MakeStatusWithDiagnostic(decl->getBeginLoc(),
+    return MakeStatusWithDiagnostic(
-                                    "returning record by value");
+        decl->getBeginLoc(), absl::StatusCode::kCancelled,
+        "returning record by value, skipping function");
   }
   const bool returns_void = return_type->isVoidType();
 
@@ -320,9 +321,10 @@ absl::StatusOr<std::string> EmitFunction(const clang::FunctionDecl* decl) {
     const clang::ParmVarDecl* param = decl->getParamDecl(i);
     if (param->getType()->isRecordType()) {
       return MakeStatusWithDiagnostic(
-          param->getBeginLoc(),
+          param->getBeginLoc(), absl::StatusCode::kCancelled,
           absl::StrCat("passing record parameter '",
-                       ToStringView(param->getName()), "' by value"));
+                       ToStringView(param->getName()),
+                       "' by value, skipping function"));
     }
 
     ParameterInfo& param_info = params.emplace_back();

sandboxed_api/tools/clang_generator/emitter_test.cc

@@ -165,34 +165,37 @@ TEST_F(EmitterTest, RemoveQualifiers) {
               ElementsAre("struct A { int data; }"));
 }
 
-TEST_F(EmitterTest, StructByValueFails) {
+TEST_F(EmitterTest, StructByValueSkipsFunction) {
   EmitterForTesting emitter;
   EXPECT_THAT(
       RunFrontendAction(
           R"(struct A { int data; };
              extern "C" int Structize(A a);)",
           std::make_unique<GeneratorAction>(emitter, GeneratorOptions())),
-      Not(IsOk()));
+      IsOk());
+  EXPECT_THAT(emitter.functions_, IsEmpty());
 }
 
-TEST_F(EmitterTest, ReturnStructByValueFails) {
+TEST_F(EmitterTest, ReturnStructByValueSkipsFunction) {
   EmitterForTesting emitter;
   EXPECT_THAT(
       RunFrontendAction(
           R"(struct A { int data; };
              extern "C" A Structize();)",
           std::make_unique<GeneratorAction>(emitter, GeneratorOptions())),
-      Not(IsOk()));
+      IsOk());
+  EXPECT_THAT(emitter.functions_, IsEmpty());
 }
 
-TEST_F(EmitterTest, TypedefStructByValueFails) {
+TEST_F(EmitterTest, TypedefStructByValueSkipsFunction) {
   EmitterForTesting emitter;
   EXPECT_THAT(
       RunFrontendAction(
           R"(typedef struct { int data; } A;
              extern "C" int Structize(A a);)",
           std::make_unique<GeneratorAction>(emitter, GeneratorOptions())),
-      Not(IsOk()));
+      IsOk());
+  EXPECT_THAT(emitter.functions_, IsEmpty());
 }
 
 TEST(IncludeGuard, CreatesRandomizedGuardForEmptyFilename) {

sandboxed_api/tools/clang_generator/generator.cc

@@ -78,6 +78,7 @@ void GeneratorASTConsumer::HandleTranslationUnit(clang::ASTContext& context) {
     ReportFatalError(context.getDiagnostics(),
                      context.getTranslationUnitDecl()->getBeginLoc(),
                      "AST traversal exited early");
+    return;
   }
 
   emitter_.AddTypesFiltered(visitor_.collector_.collected());
@@ -85,10 +86,13 @@ void GeneratorASTConsumer::HandleTranslationUnit(clang::ASTContext& context) {
   for (clang::FunctionDecl* func : visitor_.functions_) {
     absl::Status status = emitter_.AddFunction(func);
     if (!status.ok()) {
-      ReportFatalError(
+      clang::SourceLocation loc =
-          context.getDiagnostics(),
+          GetDiagnosticLocationFromStatus(status).value_or(func->getBeginLoc());
-          GetDiagnosticLocationFromStatus(status).value_or(func->getBeginLoc()),
+      if (absl::IsCancelled(status)) {
-          status.message());
+        ReportWarning(context.getDiagnostics(), loc, status.message());
+        continue;
+      }
+      ReportFatalError(context.getDiagnostics(), loc, status.message());
       break;
     }
   }