C++: Fix crash with bad filetypes in compilation outputs

#4570

RELNOTES:none
PiperOrigin-RevId: 245947851

src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcModule.java

@@ -142,13 +142,8 @@ public CcLinkingOutputs link(
 
   @Override
   public CcCompilationOutputs createCompilationOutputsFromSkylark(
-      Object objectsObject, Object picObjectsObject) {
-    CcCompilationOutputs.Builder ccCompilationOutputsBuilder = CcCompilationOutputs.builder();
-    ccCompilationOutputsBuilder.addObjectFiles(
-        convertSkylarkListOrNestedSetToNestedSet(objectsObject, Artifact.class));
-    ccCompilationOutputsBuilder.addPicObjectFiles(
-        convertSkylarkListOrNestedSetToNestedSet(picObjectsObject, Artifact.class));
-    return ccCompilationOutputsBuilder.build();
+      Object objectsObject, Object picObjectsObject, Location location) throws EvalException {
+    return super.createCompilationOutputsFromSkylark(objectsObject, picObjectsObject, location);
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java

@@ -67,7 +67,7 @@
 import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
 import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
 import com.google.devtools.build.lib.syntax.SkylarkType;
-import com.google.devtools.build.lib.util.FileType;
+import com.google.devtools.build.lib.util.FileTypeSet;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.StringUtil;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -1491,15 +1491,19 @@ protected Tuple<Object> compile(
         "srcs",
         sources,
         CppFileTypes.ALL_C_CLASS_SOURCE,
-        FileType.of(
-            ImmutableList.<String>builder()
-                .addAll(CppFileTypes.CPP_SOURCE.getExtensions())
-                .addAll(CppFileTypes.C_SOURCE.getExtensions())
-                .build()));
+        FileTypeSet.of(CppFileTypes.CPP_SOURCE, CppFileTypes.C_SOURCE));
     validateExtensions(
-        location, "public_hdrs", publicHeaders, CppFileTypes.CPP_HEADER, CppFileTypes.CPP_HEADER);
+        location,
+        "public_hdrs",
+        publicHeaders,
+        FileTypeSet.of(CppFileTypes.CPP_HEADER),
+        FileTypeSet.of(CppFileTypes.CPP_HEADER));
     validateExtensions(
-        location, "private_hdrs", privateHeaders, CppFileTypes.CPP_HEADER, CppFileTypes.CPP_HEADER);
+        location,
+        "private_hdrs",
+        privateHeaders,
+        FileTypeSet.of(CppFileTypes.CPP_HEADER),
+        FileTypeSet.of(CppFileTypes.CPP_HEADER));
 
     CcCompilationHelper helper =
         new CcCompilationHelper(
@@ -1625,15 +1629,31 @@ protected CcLinkingOutputs link(
     }
   }
 
+  protected CcCompilationOutputs createCompilationOutputsFromSkylark(
+      Object objectsObject, Object picObjectsObject, Location location) throws EvalException {
+    CcCompilationOutputs.Builder ccCompilationOutputsBuilder = CcCompilationOutputs.builder();
+    NestedSet<Artifact> objects =
+        convertSkylarkListOrNestedSetToNestedSet(objectsObject, Artifact.class);
+    validateExtensions(
+        location, "objects", objects.toList(), Link.OBJECT_FILETYPES, Link.OBJECT_FILETYPES);
+    NestedSet<Artifact> picObjects =
+        convertSkylarkListOrNestedSetToNestedSet(picObjectsObject, Artifact.class);
+    validateExtensions(
+        location, "pic_objects", picObjects.toList(), Link.OBJECT_FILETYPES, Link.OBJECT_FILETYPES);
+    ccCompilationOutputsBuilder.addObjectFiles(objects);
+    ccCompilationOutputsBuilder.addPicObjectFiles(picObjects);
+    return ccCompilationOutputsBuilder.build();
+  }
+
   private void validateExtensions(
       Location location,
       String paramName,
       List<Artifact> files,
-      FileType validFileType,
-      FileType fileTypeForErrorMessage)
+      FileTypeSet validFileTypeSet,
+      FileTypeSet fileTypeForErrorMessage)
       throws EvalException {
     for (Artifact file : files) {
-      if (!validFileType.matches(file)) {
+      if (!validFileTypeSet.matches(file.getFilename())) {
         throw new EvalException(
             location,
             String.format(

src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java

@@ -29,14 +29,12 @@ public final class CppFileTypes {
   public static final FileType C_SOURCE = FileType.of(".c");
   public static final FileType OBJC_SOURCE = FileType.of(".m");
   public static final FileType OBJCPP_SOURCE = FileType.of(".mm");
-  public static final FileType ALL_C_CLASS_SOURCE =
-      FileType.of(
-          FileTypeSet.of(
-                  CppFileTypes.CPP_SOURCE,
-                  CppFileTypes.C_SOURCE,
-                  CppFileTypes.OBJCPP_SOURCE,
-                  CppFileTypes.OBJC_SOURCE)
-              .getExtensions());
+  public static final FileTypeSet ALL_C_CLASS_SOURCE =
+      FileTypeSet.of(
+          CppFileTypes.CPP_SOURCE,
+          CppFileTypes.C_SOURCE,
+          CppFileTypes.OBJCPP_SOURCE,
+          CppFileTypes.OBJC_SOURCE);
 
   // Filetypes that generate LLVM bitcode when -flto is specified.
   public static final FileTypeSet LTO_SOURCE =

src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/BazelCcModuleApi.java

@@ -319,6 +319,7 @@ LinkingOutputsT link(
   @SkylarkCallable(
       name = "create_compilation_outputs",
       doc = "Create compilation outputs object.",
+      useLocation = true,
       parameters = {
         @Param(
             name = "objects",
@@ -344,7 +345,7 @@ LinkingOutputsT link(
             }),
       })
   CompilationOutputsT createCompilationOutputsFromSkylark(
-      Object objectsObject, Object picObjectsObject);
+      Object objectsObject, Object picObjectsObject, Location location) throws EvalException;
 
   @SkylarkCallable(
       name = "merge_compilation_outputs",

src/main/java/com/google/devtools/build/skydoc/fakebuildapi/cpp/FakeCcModule.java

@@ -259,7 +259,7 @@ public CcToolchainConfigInfoApi ccToolchainConfigInfoFromSkylark(
 
   @Override
   public CcCompilationOutputsApi<FileApi> createCompilationOutputsFromSkylark(
-      Object objectsObject, Object picObjectsObject) {
+      Object objectsObject, Object picObjectsObject, Location location) {
     return null;
   }
 

src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java

@@ -4642,27 +4642,7 @@ public void testWrongExtensionThrowsError() throws Exception {
 
   @Test
   public void testCcOutputsMerging() throws Exception {
-    scratch.overwriteFile("tools/build_defs/foo/BUILD");
-    scratch.file(
-        "tools/build_defs/foo/extension.bzl",
-        "load('//myinfo:myinfo.bzl', 'MyInfo')",
-        "def _cc_skylark_library_impl(ctx):",
-        "    c1 = cc_common.create_compilation_outputs(objects=depset([ctx.file.object1]),",
-        "        pic_objects=depset([ctx.file.pic_object1]))",
-        "    c2 = cc_common.create_compilation_outputs(objects=depset([ctx.file.object2]),",
-        "        pic_objects=depset([ctx.file.pic_object2]))",
-        "    compilation_outputs = cc_common.merge_compilation_outputs(",
-        "        compilation_outputs=[c1, c2])",
-        "    return [MyInfo(compilation_outputs=compilation_outputs)]",
-        "cc_skylark_library = rule(",
-        "    implementation = _cc_skylark_library_impl,",
-        "    attrs = {",
-        "      'object1': attr.label(allow_single_file=True),",
-        "      'pic_object1': attr.label(allow_single_file=True),",
-        "      'object2': attr.label(allow_single_file=True),",
-        "      'pic_object2': attr.label(allow_single_file=True),",
-        "    },",
-        ")");
+    setupCcOutputsTest();
     scratch.file(
         "foo/BUILD",
         "load('//tools/build_defs/foo:extension.bzl', 'cc_skylark_library')",
@@ -4687,6 +4667,26 @@ public void testCcOutputsMerging() throws Exception {
         .containsExactly("src foo/object1.o", "src foo/object2.o");
   }
 
+  @Test
+  public void testObjectsWrongExtension() throws Exception {
+    doTestCcOutputsWrongExtension("object1", "objects");
+  }
+
+  @Test
+  public void testPicObjectsWrongExtension() throws Exception {
+    doTestCcOutputsWrongExtension("pic_object1", "pic_objects");
+  }
+
+  @Test
+  public void testObjectsRightExtension() throws Exception {
+    doTestCcOutputsRightExtension("object1");
+  }
+
+  @Test
+  public void testPicObjectsRightExtension() throws Exception {
+    doTestCcOutputsRightExtension("pic_object1");
+  }
+
   @Test
   public void testCreateOnlyPic() throws Exception {
     getAnalysisMock()
@@ -4939,66 +4939,34 @@ public void testAdditionalInputs() throws Exception {
 
   @Test
   public void testPossibleSrcsExtensions() throws Exception {
-    doTestPossibleExtensions("srcs", CppFileTypes.ALL_C_CLASS_SOURCE.getExtensions());
+    doTestPossibleExtensionsOfSrcsAndHdrs("srcs", CppFileTypes.ALL_C_CLASS_SOURCE.getExtensions());
   }
 
   @Test
   public void testPossiblePrivateHdrExtensions() throws Exception {
-    doTestPossibleExtensions("private_hdrs", CppFileTypes.CPP_HEADER.getExtensions());
+    doTestPossibleExtensionsOfSrcsAndHdrs("private_hdrs", CppFileTypes.CPP_HEADER.getExtensions());
   }
 
   @Test
   public void testPossiblePublicHdrExtensions() throws Exception {
-    doTestPossibleExtensions("public_hdrs", CppFileTypes.CPP_HEADER.getExtensions());
-  }
-
-  private void doTestPossibleExtensions(String attrName, List<String> extensions) throws Exception {
-    createFiles(scratch, "tools/build_defs/foo");
-    reporter.removeHandler(failFastHandler);
-
-    for (String extension : extensions) {
-      scratch.deleteFile("bar/BUILD");
-      scratch.file(
-          "bar/BUILD",
-          "load('//tools/build_defs/foo:extension.bzl', 'cc_skylark_library')",
-          "cc_skylark_library(",
-          "    name = 'skylark_lib',",
-          "    " + attrName + " = ['file" + extension + "'],",
-          ")");
-      getConfiguredTarget("//bar:skylark_lib");
-      assertNoEvents();
-    }
+    doTestPossibleExtensionsOfSrcsAndHdrs("public_hdrs", CppFileTypes.CPP_HEADER.getExtensions());
   }
 
   @Test
   public void testWrongSrcsExtensionGivesError() throws Exception {
-    doTestWrongExtension("srcs");
+    doTestWrongExtensionOfSrcsAndHdrs("srcs");
   }
 
   @Test
   public void testWrongPrivateHdrExtensionGivesError() throws Exception {
-    doTestWrongExtension("private_hdrs");
+    doTestWrongExtensionOfSrcsAndHdrs("private_hdrs");
   }
 
   @Test
   public void testWrongPublicHdrExtensionGivesError() throws Exception {
-    doTestWrongExtension("public_hdrs");
+    doTestWrongExtensionOfSrcsAndHdrs("public_hdrs");
   }
 
-  private void doTestWrongExtension(String attrName) throws Exception {
-    createFiles(scratch, "tools/build_defs/foo");
-    scratch.file(
-        "bar/BUILD",
-        "load('//tools/build_defs/foo:extension.bzl', 'cc_skylark_library')",
-        "cc_skylark_library(",
-        "    name = 'skylark_lib',",
-        "    " + attrName + " = ['skylark_lib.cannotpossiblybevalid'],",
-        ")");
-    reporter.removeHandler(failFastHandler);
-    getConfiguredTarget("//bar:skylark_lib");
-    assertContainsEvent(
-        "has wrong extension. The list of possible extensions for '" + attrName + "'");
-  }
 
   @Test
   public void testWrongSrcExtensionGivesError() throws Exception {
@@ -5140,4 +5108,107 @@ private static void createFiles(
         "    deps = ['skylark_lib'],",
         ")");
   }
+
+  private void doTestWrongExtensionOfSrcsAndHdrs(String attrName) throws Exception {
+    createFiles(scratch, "tools/build_defs/foo");
+    scratch.file(
+        "bar/BUILD",
+        "load('//tools/build_defs/foo:extension.bzl', 'cc_skylark_library')",
+        "cc_skylark_library(",
+        "    name = 'skylark_lib',",
+        "    " + attrName + " = ['skylark_lib.cannotpossiblybevalid'],",
+        ")");
+    reporter.removeHandler(failFastHandler);
+    getConfiguredTarget("//bar:skylark_lib");
+    assertContainsEvent(
+        "has wrong extension. The list of possible extensions for '" + attrName + "'");
+  }
+
+  private void doTestPossibleExtensionsOfSrcsAndHdrs(String attrName, List<String> extensions)
+      throws Exception {
+    createFiles(scratch, "tools/build_defs/foo");
+    reporter.removeHandler(failFastHandler);
+
+    for (String extension : extensions) {
+      scratch.deleteFile("bar/BUILD");
+      scratch.file(
+          "bar/BUILD",
+          "load('//tools/build_defs/foo:extension.bzl', 'cc_skylark_library')",
+          "cc_skylark_library(",
+          "    name = 'skylark_lib',",
+          "    " + attrName + " = ['file" + extension + "'],",
+          ")");
+      getConfiguredTarget("//bar:skylark_lib");
+      assertNoEvents();
+    }
+  }
+
+  private void doTestCcOutputsWrongExtension(String attrName, String paramName) throws Exception {
+    setupCcOutputsTest();
+    scratch.file(
+        "foo/BUILD",
+        "load('//tools/build_defs/foo:extension.bzl', 'cc_skylark_library')",
+        "cc_skylark_library(",
+        "    name = 'skylark_lib',",
+        "    " + attrName + " = 'object.cannotpossiblybevalid',",
+        ")");
+    reporter.removeHandler(failFastHandler);
+    getConfiguredTarget("//foo:skylark_lib");
+    assertContainsEvent(
+        "has wrong extension. The list of possible extensions for '" + paramName + "'");
+  }
+
+  private void doTestCcOutputsRightExtension(String paramName) throws Exception {
+    setupCcOutputsTest();
+    reporter.removeHandler(failFastHandler);
+
+    for (String extension : Link.OBJECT_FILETYPES.getExtensions()) {
+      scratch.deleteFile("foo/BUILD");
+      scratch.file(
+          "foo/BUILD",
+          "load('//tools/build_defs/foo:extension.bzl', 'cc_skylark_library')",
+          "cc_skylark_library(",
+          "    name = 'skylark_lib',",
+          "    " + paramName + " = 'object1" + extension + "',",
+          ")");
+      getConfiguredTarget("//foo:skylark_lib");
+      assertNoEvents();
+    }
+  }
+
+  private void setupCcOutputsTest() throws Exception {
+    scratch.overwriteFile("tools/build_defs/foo/BUILD");
+    scratch.file(
+        "tools/build_defs/foo/extension.bzl",
+        "load('//myinfo:myinfo.bzl', 'MyInfo')",
+        "def _cc_skylark_library_impl(ctx):",
+        "    objects = []",
+        "    pic_objects = []",
+        "    if ctx.file.object1 != None:",
+        "        objects.append(ctx.file.object1)",
+        "    if ctx.file.pic_object1 != None:",
+        "        pic_objects.append(ctx.file.pic_object1)",
+        "    c1 = cc_common.create_compilation_outputs(objects=depset(objects),",
+        "        pic_objects=depset(pic_objects))",
+        "    objects = []",
+        "    pic_objects = []",
+        "    if ctx.file.object2 != None:",
+        "        objects.append(ctx.file.object2)",
+        "    if ctx.file.pic_object2 != None:",
+        "        pic_objects.append(ctx.file.pic_object2)",
+        "    c2 = cc_common.create_compilation_outputs(objects=depset(objects),",
+        "        pic_objects=depset(pic_objects))",
+        "    compilation_outputs = cc_common.merge_compilation_outputs(",
+        "        compilation_outputs=[c1, c2])",
+        "    return [MyInfo(compilation_outputs=compilation_outputs)]",
+        "cc_skylark_library = rule(",
+        "    implementation = _cc_skylark_library_impl,",
+        "    attrs = {",
+        "      'object1': attr.label(allow_single_file=True),",
+        "      'pic_object1': attr.label(allow_single_file=True),",
+        "      'object2': attr.label(allow_single_file=True),",
+        "      'pic_object2': attr.label(allow_single_file=True),",
+        "    },",
+        ")");
+  }
 }