7903672: Some issues with missing dependency errors

src/main/java/org/openjdk/jextract/JextractTool.java

@@ -32,6 +32,7 @@
 import org.openjdk.jextract.impl.IncludeFilter;
 import org.openjdk.jextract.impl.IncludeHelper;
 import org.openjdk.jextract.impl.Logger;
+import org.openjdk.jextract.impl.MissingDepChecker;
 import org.openjdk.jextract.impl.NameMangler;
 import org.openjdk.jextract.impl.Options.Library;
 import org.openjdk.jextract.impl.OutputFactory;
@@ -121,10 +122,13 @@ private static List<JavaSourceFile> generateInternal(Declaration.Scoped decl, St
                                                          List<Options.Library> libs, boolean useSystemLoadLibrary,
                                                          Logger logger) {
         var transformedDecl = Stream.of(decl)
-                .map(new IncludeFilter(includeHelper, logger)::scan)
+                // process phases that add Skips first
+                .map(new IncludeFilter(includeHelper)::scan)
                 .map(new DuplicateFilter()::scan)
-                .map(new NameMangler(headerName)::scan)
                 .map(new UnsupportedFilter(logger)::scan)
+                // then do the rest
+                .map(new MissingDepChecker(logger)::scan)
+                .map(new NameMangler(headerName)::scan)
                 .findFirst().get();
         return logger.hasErrors() ?
                 List.of() :

src/main/java/org/openjdk/jextract/impl/IncludeFilter.java

@@ -34,17 +34,12 @@
  */
 public final class IncludeFilter implements Declaration.Visitor<Void, Declaration> {
     private final IncludeHelper includeHelper;
-    private final Logger logger;
 
-    public IncludeFilter(IncludeHelper includeHelper, Logger logger) {
+    public IncludeFilter(IncludeHelper includeHelper) {
         this.includeHelper = includeHelper;
-        this.logger = logger;
     }
 
     public Declaration.Scoped scan(Declaration.Scoped header) {
-        // Process all header declarations are collect potential
-        // declarations that will go into transformed HeaderTree
-        // into the this.decls field.
         header.members().forEach(fieldTree -> fieldTree.accept(this, null));
         return header;
     }
@@ -64,8 +59,6 @@ public Void visitFunction(Declaration.Function funcTree, Declaration parent) {
             //skip
             Skip.with(funcTree);
         }
-        warnMissingDep(funcTree, funcTree.type().returnType());
-        funcTree.type().argumentTypes().forEach(p -> warnMissingDep(funcTree, p));
         return null;
     }
 
@@ -90,7 +83,6 @@ public Void visitTypedef(Declaration.Typedef tree, Declaration parent) {
             //skip
             Skip.with(tree);
         }
-        warnMissingDep(tree, tree.type());
         return null;
     }
 
@@ -100,26 +92,11 @@ public Void visitVariable(Declaration.Variable tree, Declaration parent) {
             //skip
             Skip.with(tree);
         }
-        warnMissingDep(parent != null ? parent : tree, tree.type());
         return null;
     }
 
     @Override
     public Void visitDeclaration(Declaration decl, Declaration parent) {
         return null;
     }
-
-    void warnMissingDep(Declaration decl, Type type) {
-        if (type instanceof Type.Declared declared) {
-            // we only have to check for missing structs because (a) pointers to missing structs can still lead
-            // to valid code and (b) missing typedefs to existing structs are resolved correctly, as typedefs are never
-            // referred to by name in the generated code (because of libclang limitations).
-            if (Skip.isPresent(declared.tree())) {
-                logger.err("jextract.bad.include", decl.name(), declared.tree().name());
-            }
-        } else if (type instanceof Type.Delegated delegated &&
-                        delegated.kind() == Delegated.Kind.TYPEDEF) {
-            warnMissingDep(decl, delegated.type());
-        }
-    }
 }

src/main/java/org/openjdk/jextract/impl/MissingDepChecker.java

@@ -0,0 +1,104 @@
+/*
+ * Copyright (c) 2024 Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package org.openjdk.jextract.impl;
+
+import org.openjdk.jextract.Declaration;
+import org.openjdk.jextract.Type;
+import org.openjdk.jextract.Type.Delegated;
+import org.openjdk.jextract.impl.DeclarationImpl.Skip;
+
+/*
+ * This visitor marks declarations to be skipped, based on --include options specified.
+ */
+public final class MissingDepChecker implements Declaration.Visitor<Void, Declaration> {
+    private final Logger logger;
+
+    public MissingDepChecker(Logger logger) {
+        this.logger = logger;
+    }
+
+    public Declaration.Scoped scan(Declaration.Scoped header) {
+        header.members().forEach(fieldTree -> fieldTree.accept(this, null));
+        return header;
+    }
+
+    @Override
+    public Void visitFunction(Declaration.Function funcTree, Declaration parent) {
+        if (Skip.isPresent(funcTree)) return null;
+
+        checkMissingDep(funcTree, funcTree.type().returnType());
+        funcTree.type().argumentTypes().forEach(p -> checkMissingDep(funcTree, p));
+        return null;
+    }
+
+    @Override
+    public Void visitScoped(Declaration.Scoped d, Declaration parent) {
+        if (Skip.isPresent(d)) return null;
+
+        d.members().forEach(fieldTree -> fieldTree.accept(this, d));
+        return null;
+    }
+
+    @Override
+    public Void visitTypedef(Declaration.Typedef tree, Declaration parent) {
+        if (Skip.isPresent(tree)) return null;
+
+        checkMissingDep(tree, tree.type());
+        return null;
+    }
+
+    @Override
+    public Void visitVariable(Declaration.Variable tree, Declaration parent) {
+        if (Skip.isPresent(tree)) return null;
+
+        if (parent != null && !Skip.isPresent(parent))  {
+            checkMissingDep(parent, tree.type());
+        } else {
+            checkMissingDep(tree, tree.type());
+        }
+        return null;
+    }
+
+    @Override
+    public Void visitDeclaration(Declaration decl, Declaration parent) {
+        return null;
+    }
+
+    void checkMissingDep(Declaration decl, Type type) {
+        if (type instanceof Type.Declared declared) {
+            // we only have to check for missing structs because (a) pointers to missing structs can still lead
+            // to valid code and (b) missing typedefs to existing structs are resolved correctly, as typedefs are never
+            // referred to by name in the generated code (because of libclang limitations).
+            if (Skip.isPresent(declared.tree())) {
+                logger.err("jextract.bad.include", decl.name(), declared.tree().name());
+            }
+        } else if (type instanceof Delegated delegated &&
+                        delegated.kind() == Delegated.Kind.TYPEDEF) {
+            checkMissingDep(decl, delegated.type());
+        } else if (type instanceof Type.Array arrayType) {
+            checkMissingDep(decl, arrayType.elementType());
+        }
+    }
+}

src/main/java/org/openjdk/jextract/impl/UnsupportedFilter.java

@@ -70,6 +70,8 @@ public Declaration.Scoped scan(Declaration.Scoped header) {
 
     @Override
     public Void visitFunction(Function funcTree, Declaration firstNamedParent) {
+        if(Skip.isPresent(funcTree)) return null;
+
         Utils.forEachNested(funcTree, s -> s.accept(this, firstNamedParent));
 
         //generate static wrapper for function
@@ -106,6 +108,8 @@ private static String fieldName(Declaration firstNamedParent, Declaration decl)
 
     @Override
     public Void visitVariable(Variable varTree, Declaration firstNamedParent) {
+        if(Skip.isPresent(varTree)) return null;
+
         Utils.forEachNested(varTree, s -> s.accept(this, varTree));
 
         Type unsupportedType = firstUnsupportedType(varTree.type(), false);
@@ -127,6 +131,8 @@ public Void visitVariable(Variable varTree, Declaration firstNamedParent) {
 
     @Override
     public Void visitScoped(Scoped scoped, Declaration firstNamedParent) {
+        if(Skip.isPresent(scoped)) return null;
+
         Type unsupportedType = firstUnsupportedType(Type.declared(scoped), false);
         if (unsupportedType != null) {
             warnSkip(scoped.name(), unsupportedType(unsupportedType));
@@ -154,6 +160,8 @@ public Void visitScoped(Scoped scoped, Declaration firstNamedParent) {
 
     @Override
     public Void visitTypedef(Typedef typedefTree, Declaration firstNamedParent) {
+        if(Skip.isPresent(typedefTree)) return null;
+
         // propagate
         if (typedefTree.type() instanceof Declared declared) {
             visitScoped(declared.tree(), null);
@@ -175,6 +183,8 @@ public Void visitTypedef(Typedef typedefTree, Declaration firstNamedParent) {
 
     @Override
     public Void visitConstant(Constant d, Declaration firstNamedParent) {
+        if(Skip.isPresent(d)) return null;
+
         Type unsupportedType = firstUnsupportedType(d.type(), false);
         String name = fieldName(firstNamedParent, d);
         if (unsupportedType != null) {

test/lib/testlib/JextractToolRunner.java

@@ -45,6 +45,7 @@
 import org.openjdk.jextract.JextractTool;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertNotEquals;
 import static org.testng.Assert.assertNotNull;
 import static org.testng.Assert.assertTrue;
@@ -133,6 +134,12 @@ public JextractResult checkContainsOutput(String expected) {
             return this;
         }
 
+        public JextractResult checkDoesNotContainOutput(String expected) {
+            Objects.requireNonNull(expected);
+            assertFalse(output.contains(expected), "Output contains string: " + expected);
+            return this;
+        }
+
         public JextractResult checkMatchesOutput(String regex) {
             Objects.requireNonNull(regex);
             assertTrue(output.trim().matches(regex), "Output does not match regex: " + regex);

test/testng/org/openjdk/jextract/test/toolprovider/includeDeps/TestBadIncludes.java

@@ -38,7 +38,8 @@ public void before() {
         Path output = getOutputFilePath("TestBadIncludes-badIncludes.h");
         Path outputH = getInputFilePath("bad_includes.h");
         result = run(output,
-        "--include-struct", "B",
+                "--include-var", "a",
+                "--include-struct", "B",
                 "--include-function", "m",
                 "--include-typedef", "T",
                 "--include-struct", "C",
@@ -58,7 +59,8 @@ public static Object[][] cases() {
             {"B",   "A" },
             {"m",   "A" },
             {"T",   "A" },
-            {"a",   "A" }
+            {"a",   "A" },
+            {"C",   "A" }
         };
     }
 }

test/testng/org/openjdk/jextract/test/toolprovider/includeDeps/TestSkippedBadIncludes.java

@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package org.openjdk.jextract.test.toolprovider.includeDeps;
+
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+import testlib.JextractToolRunner;
+
+import java.nio.file.Path;
+
+public class TestSkippedBadIncludes extends JextractToolRunner {
+
+    @Test
+    public void run() {
+        Path output = getOutputFilePath("TestBadIncludes-badIncludes.h");
+        Path outputH = getInputFilePath("bad_includes.h");
+        JextractResult result = run(output,
+                // some random includes so that we don't include everything
+                "--include-function", "n",
+                outputH.toString());
+        // if nothing that depends on struct A is included
+        // there should be no errors
+        result.checkSuccess();
+    }
+}

test/testng/org/openjdk/jextract/test/toolprovider/includeDeps/bad_includes.h

@@ -26,3 +26,4 @@ struct B { struct A a; };
 void m(struct A a);
 typedef struct A T;
 struct A a;
+struct C { int x; struct A arr[]; };

test/testng/org/openjdk/jextract/test/toolprovider/unsupported/TestSkipped.java

@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package org.openjdk.jextract.test.toolprovider.unsupported;
+
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+import testlib.JextractToolRunner;
+
+import java.nio.file.Path;
+
+public class TestSkipped extends JextractToolRunner {
+
+    JextractResult result;
+
+    @BeforeClass
+    public void before() {
+        Path output = getOutputFilePath("TestUnsupportedTypes-unsupportedTypes.h");
+        Path outputH = getInputFilePath("unsupportedTypes.h");
+        result = run(output,
+                // dummy include to turn on exclusions
+                "--include-function", "nonexistent",
+                outputH.toString());
+    }
+
+    @Test(dataProvider = "cases")
+    public void testUnsupportedTypes(String skippedName) {
+        result.checkDoesNotContainOutput("WARNING: Skipping " + skippedName);
+    }
+
+    @DataProvider
+    public static Object[][] cases() {
+        return new Object[][]{
+            {"returns_unsupported"              },
+            {"accepts_unsupported"              },
+            {"unsupported_t"                    },
+            {"unsupported_func_t"               },
+            {"returns_unsupported_func"         },
+            {"accepts_unsupported_func"         },
+            {"accepts_unsupported_func_varargs" },
+            {"GLOBAL_UNSUPPORTED"               },
+            {"GLOBAL_UNSUPPORTED_FUNC"          },
+            {"accepts_undefined"                },
+            {"returns_undefined"                },
+            {"accepts_undefined_func"           },
+            {"GLOBAL_UNDECLARED"                },
+            {"undefined_typedef"                },
+            {"INT_128_NUM"                      }
+        };
+    }
+}