WIP workwip/pwithnall/unknown

6 files changed, 185 insertions, 47 deletions

diff --git a/clang-plugin/nullability-checker.cpp b/clang-plugin/nullability-checker.cpp
index ce3d47a..fc8ad2b 100644
--- a/clang-plugin/nullability-checker.cpp
+++ b/clang-plugin/nullability-checker.cpp
@@ -82,6 +82,65 @@ NullabilityConsumer::HandleTranslationUnit (ASTContext& context)
 	this->_visitor.TraverseDecl (context.getTranslationUnitDecl ());
 }
 
+static bool
+arg_is_closure (GIArgInfo arg, GITypeInfo type_info)
+{
+	return (g_arg_info_get_closure (&arg) != -1 ||
+	        g_arg_info_get_destroy (&arg) != -1 ||
+	        g_arg_info_get_scope (&arg) != GI_SCOPE_TYPE_INVALID);
+}
+
+static bool
+arg_is_cancellable (GIArgInfo arg, GITypeInfo type_info)
+{
+	bool retval;
+
+	if (g_type_info_get_tag (&type_info) != GI_TYPE_TAG_INTERFACE) {
+		return FALSE;
+	}
+
+	GIBaseInfo *iface = g_type_info_get_interface (&type_info);
+
+	retval = (g_base_info_get_type (iface) == GI_INFO_TYPE_OBJECT &&
+	          strcmp (g_base_info_get_name (iface), "Cancellable") == 0 &&
+	          strcmp (g_base_info_get_namespace (iface), "Gio") == 0);
+
+	g_base_info_unref (iface);
+
+	return retval;
+}
+
+static bool
+arg_is_async_ready_callback (GIArgInfo arg, GITypeInfo type_info)
+{
+	bool retval;
+
+	if (g_type_info_get_tag (&type_info) != GI_TYPE_TAG_INTERFACE) {
+		return FALSE;
+	}
+
+	GIBaseInfo *iface = g_type_info_get_interface (&type_info);
+
+	retval = (g_base_info_get_type (iface) == GI_INFO_TYPE_CALLBACK &&
+	          strcmp (g_base_info_get_name (iface), "AsyncReadyCallback") == 0 &&
+	          strcmp (g_base_info_get_namespace (iface), "Gio") == 0);
+
+	g_base_info_unref (iface);
+
+	return retval;
+}
+
+static bool
+arg_may_be_null (GIArgInfo arg, GITypeInfo type_info)
+{
+	return (g_arg_info_may_be_null (&arg) ||
+	        g_arg_info_is_optional (&arg) ||
+	        arg_is_closure (arg, type_info) ||
+	        (g_arg_info_get_direction (&arg) != GI_DIRECTION_OUT &&
+	         (arg_is_cancellable (arg, type_info) ||
+	          arg_is_async_ready_callback (arg, type_info))));
+}
+
 /* Note: Specifically overriding the Traverse* method here to re-implement
  * recursion to child nodes. */
 bool
@@ -171,32 +230,45 @@ NullabilityVisitor::TraverseFunctionDecl (FunctionDecl* func)
 
 	GICallableInfo *callable_info = (GICallableInfo *) info;
 
+	/* GError formal parameters aren’t included in the number of
+	 * callable arguments. */
+	unsigned int k = g_callable_info_get_n_args (callable_info);
+	unsigned int err_params =
+		(g_function_info_get_flags (callable_info) &
+		 GI_FUNCTION_THROWS) ? 1 : 0;
+	unsigned int obj_params =
+		(g_base_info_get_container (info) != NULL &&
+		 g_function_info_get_flags (callable_info) &
+		 GI_FUNCTION_IS_METHOD) ? 1 : 0;
+	unsigned int j;
+
 	/* Handle the parameters. */
-	for (FunctionDecl::param_const_iterator it = func->param_begin (),
-	     ie = func->param_end (); it != ie; ++it) {
-		ParmVarDecl* parm_decl = *it;
-		unsigned int idx = parm_decl->getFunctionScopeIndex ();
+	for (j = 0; j < k; j++) {
 		GIArgInfo arg;
 		GITypeInfo type_info;
+		GITransfer transfer;
+		GITypeTag type_tag;
+		ParmVarDecl *parm = func->getParamDecl (obj_params + j);
+
+		g_callable_info_load_arg (callable_info, j, &arg);
+		g_arg_info_load_type (&arg, &type_info);
+		transfer = g_arg_info_get_ownership_transfer (&arg);
+		type_tag = g_type_info_get_tag (&type_info);
 
 		/* Skip non-pointer arguments. */
-		if (!parm_decl->getType ()->isPointerType ())
+		if (!parm->getType ()->isPointerType ())
 			continue;
 
-		g_callable_info_load_arg (callable_info, idx, &arg);
-		g_arg_info_load_type (&arg, &type_info);
-
 		enum {
 			EXPLICIT_NULLABLE,  /* 0 */
 			MAYBE,  /* ? */
 			EXPLICIT_NONNULL,  /* 1 */
 		} has_nonnull =
 			(nonnull_attr == NULL) ? MAYBE :
-			(nonnull_attr->isNonNull (idx)) ?
+			(nonnull_attr->isNonNull (obj_params + j)) ?
 				EXPLICIT_NONNULL: EXPLICIT_NULLABLE;
-		bool has_nullable = (g_arg_info_may_be_null (&arg) ||
-		                     g_arg_info_is_optional (&arg));
-		bool has_assertion = (asserted_parms.count (parm_decl) > 0);
+		bool has_nullable = arg_may_be_null (arg, type_info);
+		bool has_assertion = (asserted_parms.count (parm) > 0);
 
 		/* Analysis:
 		 *
@@ -236,8 +308,8 @@ NullabilityVisitor::TraverseFunctionDecl (FunctionDecl* func)
 				"annotation on the ‘%0’ parameter "
 				"of function %1().",
 				this->_compiler,
-				parm_decl->getLocStart ())
-			<< parm_decl->getNameAsString ()
+				parm->getLocStart ())
+			<< parm->getNameAsString ()
 			<< func->getNameAsString ();
 		} else if (has_nullable && has_assertion) {
 			Debug::emit_error (
@@ -246,8 +318,8 @@ NullabilityVisitor::TraverseFunctionDecl (FunctionDecl* func)
 				"non-NULL precondition assertion on the ‘%0’ "
 				"parameter of function %1().",
 				this->_compiler,
-				parm_decl->getLocStart ())
-			<< parm_decl->getNameAsString ()
+				parm->getLocStart ())
+			<< parm->getNameAsString ()
 			<< func->getNameAsString ();
 		} else if (!has_nullable && !has_assertion) {
 			switch (has_nonnull) {
@@ -259,8 +331,8 @@ NullabilityVisitor::TraverseFunctionDecl (FunctionDecl* func)
 					"(already has a nonnull attribute or "
 					"no non-NULL precondition assertion).",
 					this->_compiler,
-					parm_decl->getLocStart ())
-				<< parm_decl->getNameAsString ()
+					parm->getLocStart ())
+				<< parm->getNameAsString ()
 				<< func->getNameAsString ();
 				break;
 			case MAYBE:
@@ -270,8 +342,8 @@ NullabilityVisitor::TraverseFunctionDecl (FunctionDecl* func)
 					"non-NULL precondition assertion on "
 					"the ‘%0’ parameter of function %1().",
 					this->_compiler,
-					parm_decl->getLocStart ())
-				<< parm_decl->getNameAsString ()
+					parm->getLocStart ())
+				<< parm->getNameAsString ()
 				<< func->getNameAsString ();
 				break;
 			case EXPLICIT_NONNULL:
@@ -283,8 +355,8 @@ NullabilityVisitor::TraverseFunctionDecl (FunctionDecl* func)
 					"(optional) or (allow-none) "
 					"annotation).",
 					this->_compiler,
-					parm_decl->getLocStart ())
-				<< parm_decl->getNameAsString ()
+					parm->getLocStart ())
+				<< parm->getNameAsString ()
 				<< func->getNameAsString ();
 				break;
 			}
@@ -294,8 +366,8 @@ NullabilityVisitor::TraverseFunctionDecl (FunctionDecl* func)
 				"non-NULL precondition annotation on the ‘%0’ "
 				"parameter of function %1().",
 				this->_compiler,
-				parm_decl->getLocStart ())
-			<< parm_decl->getNameAsString ()
+				parm->getLocStart ())
+			<< parm->getNameAsString ()
 			<< func->getNameAsString ();
 		} else if (has_nonnull == MAYBE && has_assertion) {
 			/* TODO: Make this a soft warning (disabled by default)
@@ -305,8 +377,8 @@ NullabilityVisitor::TraverseFunctionDecl (FunctionDecl* func)
 				"parameter of function %1() (already has a "
 				"non-NULL precondition assertion).",
 				this->_compiler,
-				parm_decl->getLocStart ())
-			<< parm_decl->getNameAsString ()
+				parm->getLocStart ())
+			<< parm->getNameAsString ()
 			<< func->getNameAsString ();
 		}
 	}
diff --git a/clang-plugin/plugin.cpp b/clang-plugin/plugin.cpp
index 4130c8c..577eb10 100644
--- a/clang-plugin/plugin.cpp
+++ b/clang-plugin/plugin.cpp
@@ -221,7 +221,8 @@ private:
 
 				std::string _typelib_filename (typelib_filename);
 				std::string::size_type last_dot = _typelib_filename.find_last_of (".");
-				if (last_dot == std::string::npos) {
+				if (last_dot == std::string::npos ||
+				    _typelib_filename.substr (last_dot) != ".typelib") {
 					/* No ‘.typelib’ suffix — ignore. */
 					continue;
 				}
@@ -243,6 +244,18 @@ protected:
 	ParseArgs (const CompilerInstance &CI,
 	           const std::vector<std::string>& args)
 	{
+		/* Check for additional GI paths. */
+		for (std::vector<std::string>::const_iterator it = args.begin();
+		     it != args.end (); ++it) {
+			std::string arg = *it;
+
+			if (arg == "--gi-path") {
+				const std::string path = *(++it);
+
+				g_irepository_prepend_search_path (path.c_str ());
+			}
+		}
+
 		/* Load all typelibs. */
 		this->_load_gi_repositories (CI);
 
@@ -321,6 +334,8 @@ protected:
 		       "        Disable the given Tartan checker, which may be "
 		               "‘all’. All checkers are\n"
 		       "        enabled by default.\n"
+		       "    --gi-path [path]\n"
+		       "        Add an additional typelib search path.\n"
 		       "    --quiet\n"
 		       "        Disable all plugin output except code "
 		               "diagnostics (remarks,\n"
diff --git a/configure.ac b/configure.ac
index 1593d27..c710767 100644
--- a/configure.ac
+++ b/configure.ac
@@ -104,6 +104,12 @@ AC_CHECK_HEADER([clang/AST/Expr.h],[],[
 CPPFLAGS="$old_cppflags"
 AC_LANG_POP([C++])
 
+# g-ir-compiler for unit tests
+AC_PATH_PROG([G_IR_COMPILER],[g-ir-compiler])
+AS_IF([test "$G_IR_COMPILER" == ""],[
+	AC_MSG_ERROR([g-ir-compiler not found])
+])
+
 # Internationalisation
 GETTEXT_PACKAGE=AC_PACKAGE_NAME
 AC_SUBST([GETTEXT_PACKAGE])
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 00c0b4f..6a78146 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -5,6 +5,7 @@
 TEST_EXTENSIONS = .c
 C_LOG_COMPILER = $(top_srcdir)/tests/wrapper-compiler-errors
 
+# c_tests are split up and have templates attached; simple_c_tests are not.
 c_tests = \
 	assertion-extraction.c \
 	assertion-extraction-return.c \
@@ -19,6 +20,12 @@ c_tests = \
 	nonnull.c \
 	gerror-api.c \
 	$(NULL)
+simple_c_tests = \
+	annotations.simple.c \
+	$(NULL)
+test_typelibs = \
+	Test-1.0.typelib \
+	$(NULL)
 
 templates = \
 	assertion.head.c \
@@ -37,11 +44,20 @@ templates = \
 	gvariant.tail.c \
 	$(NULL)
 
-TESTS = $(c_tests)
+TESTS = $(c_tests) $(simple_c_tests)
 EXTRA_DIST = \
 	$(templates) \
 	$(c_tests) \
+	$(simple_c_tests) \
 	wrapper-compiler-errors \
 	$(NULL)
 
+# Test GIR file and typelib which we can load.
+%.typelib: %.gir
+	$(AM_V_GEN) $(G_IR_COMPILER) --includedir=. $< -o $@
+
+EXTRA_DIST += Test-1.0.gir
+BUILT_SOURCES = $(test_typelibs)
+CLEANFILES = $(test_typelibs)
+
 -include $(top_srcdir)/git.mk
diff --git a/tests/annotations.simple.c b/tests/annotations.simple.c
new file mode 100644
index 0000000..0a84f10
--- /dev/null
+++ b/tests/annotations.simple.c
@@ -0,0 +1,20 @@
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <glib.h>
+#include <gio/gio.h>
+
+/*
+ * No error
+ */
+void
+g_input_stream_read_async (GInputStream *stream,
+                           void *buffer,
+                           gsize count,
+                           int io_priority,
+                           GCancellable *cancellable,
+                           GAsyncReadyCallback callback,
+                           gpointer user_data)
+{
+	// Do nothing.
+}
diff --git a/tests/wrapper-compiler-errors b/tests/wrapper-compiler-errors
index 8e30771..df43990 100755
--- a/tests/wrapper-compiler-errors
+++ b/tests/wrapper-compiler-errors
@@ -25,6 +25,7 @@ temp_dir=`mktemp -d`
 tests_dir=`dirname $0`
 tartan=${tests_dir}/../scripts/tartan
 tartan_plugin=${tests_dir}/../clang-plugin/.libs/libtartan.so
+simple_extension="${input_filename:(-9)}"
 
 echo "Reading input from ${input_filename}."
 echo "Using temporary directory ${temp_dir}."
@@ -38,21 +39,27 @@ test_status=0
 system_includes=`echo | cpp -Wp,-v 2>&1 | grep '^[[:space:]]' | \
 	sed -e 's/^[[:space:]]*/-isystem/' | tr "\n" ' '`
 
-# Extract the template name.
-template_name=`head -n 1 "${input_filename}" | \
-	sed -n 's/\/\*[[:space:]]*Template:\(.*\)\*\//\1/p' | \
-	tr -d ' '`
+if [[ $simple_extension != ".simple.c" ]]; then
+	# Extract the template name.
+	template_name=`head -n 1 "${input_filename}" | \
+		sed -n 's/\/\*[[:space:]]*Template:\(.*\)\*\//\1/p' | \
+		tr -d ' '`
 
-echo "Using template ${template_name}."
+	echo "Using template ${template_name}."
 
-# Split the input file up into sections, delimiting on ‘/*’ on a line by itself.
-tail -n +3 "${input_filename}" > "${temp_dir}/${input_filename}.tail"
-csplit --keep-files --elide-empty-files --silent \
-	--prefix="${temp_dir}/${input_filename}_" \
-	--suffix-format='%02d.c' \
-	"${temp_dir}/${input_filename}.tail" '/^\/\*/' '{*}'
+	# Split the input file up into sections, delimiting on ‘/*’ on a line by itself.
+	tail -n +3 "${input_filename}" > "${temp_dir}/${input_filename}.tail"
+	csplit --keep-files --elide-empty-files --silent \
+		--prefix="${temp_dir}/${input_filename}_" \
+		--suffix-format='%02d.c' \
+		"${temp_dir}/${input_filename}.tail" '/^\/\*/' '{*}'
 
-echo ""
+	echo ""
+else
+	# Don’t split simple input files up.
+	echo "Copying file unmodified."
+	cp "${input_filename}" "${temp_dir}/${input_filename}_00.c"
+fi
 
 num=0
 while [[ -f `printf "${temp_dir}/${input_filename}_%02d.c" ${num}` ]]; do
@@ -66,12 +73,14 @@ while [[ -f `printf "${temp_dir}/${input_filename}_%02d.c" ${num}` ]]; do
 	echo " - Building section file ${section_filename}."
 	echo " - Outputting to error files ${expected_error_filename} and ${actual_error_filename}."
 
-	# Wrap the section’s code with a prefix and suffix.
-	(cat "${template_name}.head.c"
-	 cat "${section_filename}"
-	 cat "${template_name}.tail.c"
-	) > $section_filename.tmp
-	mv -f $section_filename.tmp $section_filename
+	if [[ $simple_extension != ".simple.c" ]]; then
+		# Wrap the section’s code with a prefix and suffix.
+		(cat "${template_name}.head.c"
+		 cat "${section_filename}"
+		 cat "${template_name}.tail.c"
+		) > $section_filename.tmp
+		mv -f $section_filename.tmp $section_filename
+	fi
 
 	num=$((num + 1))
 
@@ -91,7 +100,7 @@ while [[ -f `printf "${temp_dir}/${input_filename}_%02d.c" ${num}` ]]; do
 	# TARTAN_TEST_OPTIONS="-analyzer-checker=debug.ViewExplodedGraph" to
 	# debug the ExplodedGraph
 	TARTAN_PLUGIN=$tartan_plugin \
-	TARTAN_OPTIONS=--quiet \
+	TARTAN_OPTIONS="--quiet --gi-path ." \
 	$tartan \
 		-cc1 -analyze -std=c89 -Wno-visibility $TARTAN_TEST_OPTIONS \
 		`pkg-config --cflags glib-2.0` \