Re: Serious bug in idl compiler (non-compiling generated files)

[Jules Colding <colding omesc com>]

On Wed, 2004-12-22 at 16:30 +0000, Michael Meeks wrote:
> On Wed, 2004-12-22 at 14:27 +0100, Jules Colding wrote:
> > > 	Also - with libIDL one of the things I've often wanted to do is to
> > > write / steal a very small/simple pre-processor & have that built-in to
> > > avoid requiring invoking gpp [ also should bin forking 'indent' to make
> > > the output look nicer, it's really not necessary ], if you're interested
> > > that is :-)
> > 
> > Yes, but somewhat time constrained :-) 
> > 
> > Could you expand on your thoughts about indent?
> 
> 	Oh - we used to produce _excessively_ huge & ugly code from orbit-idl,
> but what's there now should be much smaller & neater. Hence we used to
> (still do?) fork indent & pipe the data through that to make it look
> nicer. I'd like to stop that process happening / hack that code out,
> it's just a source of problems / errors I think.

OK, I had half an hour to kill, so here is a patch that removes support
for the forking of an external indentation process. The resulting IDL
compiler is a *lot* faster BTW. Comments?

-- 
  jules


diff -urpN ORBit2.orig/ChangeLog ORBit2/ChangeLog
--- ORBit2.orig/ChangeLog	2004-12-27 10:13:20.052632490 +0100
+++ ORBit2/ChangeLog	2004-12-27 12:59:07.657521294 +0100
@@ -1,3 +1,16 @@
+2004-12-27  Jules Colding  <colding omesc com>
+
+	* configure.in: Remove support for external indentation command.
+
+	* src/idl-compiler/orbit-idl-main.c: Removed all traces of the output formatter 
+	command line option.
+
+	* src/idl-compiler/orbit-idl-c-backend.c (out_for_pass): "popen ==> fopen" for 
+	all output files. The reason is to avoid opening a pipe as we don't want to use
+	an externally invoked indentation process.
+
+	* src/idl-compiler/orbit-idl3-types.h: Removed output_formatter.
+
 2004-12-22  Jules Colding  <colding omesc com>
 
 	* test/everything/everything.idl: Added test for bug#161879.
diff -urpN ORBit2.orig/configure.in ORBit2/configure.in
--- ORBit2.orig/configure.in	2004-12-27 10:13:20.048632955 +0100
+++ ORBit2/configure.in	2004-12-27 12:30:16.092913511 +0100
@@ -71,35 +71,6 @@ else
 fi
 AC_SUBST(ORBIT_DEBUG_CFLAGS)
 
-dnl Handle indent variations 
-dnl default is "sopwith indent", use "--enable-indent=kr" for "kr"appy indent.
-AC_ARG_ENABLE(indent, [  --disable-indent        do not indent output code], use_indent=$enableval, use_indent=yes)
-
-AC_MSG_CHECKING([for indentation command to pipe generated c-files through])
-for test_indent in "$use_indent" "indent -" cat; do
-  case "$test_indent" in
-    yes)
-      INDENT_COMMAND="indent -npro -bad -bap -bc -sob -br -ce -cli2 -npcs -di1 -psl -i3 -lp -st"
-      ;;
-    no)
-      INDENT_COMMAND=cat
-      ;;
-    kr)
-      INDENT_COMMAND="indent -npro -kr -i8 -bad -fca -sc -sob -psl"
-      ;;
-    *)
-      INDENT_COMMAND="$test_indent"
-  esac
-
-  if echo 2>/dev/null | $INDENT_COMMAND 2>/dev/null 1>&2 ; then
-    break
-  fi
-done
-AC_MSG_RESULT($INDENT_COMMAND)
-AC_DEFINE_UNQUOTED(INDENT_COMMAND,"$INDENT_COMMAND",
-                   [command used for indentation])
-
-
 AC_ARG_ENABLE(purify, [  --enable-purify        be clean for memory debuggers], enable_purify=yes, enable_purify=no)
 if test z$enable_purify = zyes; then
 	AC_DEFINE(ORBIT_PURIFY, 1, [defined if purify is enabled])
diff -urpN ORBit2.orig/src/idl-compiler/orbit-idl3-types.h ORBit2/src/idl-compiler/orbit-idl3-types.h
--- ORBit2.orig/src/idl-compiler/orbit-idl3-types.h	2004-12-27 10:13:19.944645042 +0100
+++ ORBit2/src/idl-compiler/orbit-idl3-types.h	2004-12-27 12:42:50.573724902 +0100
@@ -29,8 +29,6 @@ typedef struct {
 	 OUTPUT_DEPS=1<<6 /* Make sure this is always the last pass or dep output will break. */
   } enabled_passes;
 
-  char *output_formatter;
-
   char *output_language;
   char *input_filename;
   char *backend_directory;
diff -urpN ORBit2.orig/src/idl-compiler/orbit-idl-c-backend.c ORBit2/src/idl-compiler/orbit-idl-c-backend.c
--- ORBit2.orig/src/idl-compiler/orbit-idl-c-backend.c	2004-12-27 10:13:19.946644809 +0100
+++ ORBit2/src/idl-compiler/orbit-idl-c-backend.c	2004-12-27 12:52:35.161394495 +0100
@@ -62,10 +62,7 @@ orbit_idl_output_c (IDL_tree       tree,
 	orbit_idl_output_c_deps(tree, rinfo, &ci);
 	break;
       }
-      if (1 << i == OUTPUT_DEPS)
-	fclose(ci.fh);
-      else 
-	pclose(ci.fh);
+      fclose(ci.fh);
     }
   }
   g_string_free(ci.ext_dcls,TRUE);
@@ -124,7 +121,6 @@ out_for_pass (const char    *input_filen
 {
 	FILE *fp;
 	char *output_filename;
-	char *cmdline;
 	gchar *output_full_path = NULL;
 
 
@@ -157,20 +153,11 @@ out_for_pass (const char    *input_filen
 		output_full_path = g_build_path (G_DIR_SEPARATOR_S, rinfo->output_directory, output_filename, NULL);
 		g_free (output_filename);
 
-		cmdline = g_alloca (strlen (rinfo->output_formatter) +
-				    strlen (output_full_path) +
-				    sizeof(" > "));
-		sprintf (cmdline, "%s > %s", rinfo->output_formatter, output_full_path);
-
+		fp = fopen (output_full_path, "w");
 		g_free (output_full_path);
 
-		/* Many versions of cpp do evil translating internal
-		* strings, producing bogus output, so clobber LC_ALL */
-		putenv ("LC_ALL=C");
-		fp = popen (cmdline, "w");
-
 		if (fp == NULL)
-			g_error ("failed to popen '%s': %s\n", cmdline, strerror(errno));
+			g_error ("failed to fopen '%s': %s\n", output_full_path, strerror(errno));
 	}
 
 	return fp;
diff -urpN ORBit2.orig/src/idl-compiler/orbit-idl-main.c ORBit2/src/idl-compiler/orbit-idl-main.c
--- ORBit2.orig/src/idl-compiler/orbit-idl-main.c	2004-12-27 10:13:19.949644461 +0100
+++ ORBit2/src/idl-compiler/orbit-idl-main.c	2004-12-27 12:42:50.555727006 +0100
@@ -58,8 +58,6 @@ static char *cl_output_directory = "";
 #define BASE_CPP_ARGS "-D__ORBIT_IDL__ "
 static GString *cl_cpp_args;
 
-static char *c_output_formatter = INDENT_COMMAND;
-
 /* Callbacks for popt */
 static void
 cl_libIDL_version_callback(poptContext con, enum poptCallbackReason reason,
@@ -126,7 +124,6 @@ struct poptOption options[] = {
   {"add-imodule", '\0', POPT_ARG_NONE, &cl_add_imodule, 0, "Output an imodule file", NULL},
   {"skeleton-impl", '\0', POPT_ARG_NONE, &cl_enable_skeleton_impl, 0, "Output skeleton implementation", NULL},
   {"backenddir", '\0', POPT_ARG_STRING, &cl_backend_dir, 0, "Override IDL backend library directory", "DIR"},
-  {"c-output-formatter", '\0', POPT_ARG_STRING, &c_output_formatter, 0, "Program to use to format output (normally, indent)", "PROGRAM"},
   {"onlytop", '\0', POPT_ARG_NONE, &cl_onlytop, 0, "Inhibit includes", NULL},
   {"pidl", '\0', POPT_ARG_NONE, &cl_is_pidl, 0, "Treat as Pseudo IDL", NULL},
   {"nodefskels", '\0', POPT_ARG_NONE, &cl_disable_defs_skels, 0, "Don't output defs for skels in header", NULL},
@@ -148,9 +145,6 @@ int main(int argc, const char *argv[])
   /* Argument parsing, etc. */
   cl_cpp_args = g_string_new("-D__ORBIT_IDL__ ");
 
-  if(getenv("C_OUTPUT_FORMATTER"))
-    c_output_formatter = getenv("C_OUTPUT_FORMATTER");
-
   pcon = poptGetContext ("orbit-idl-2", argc, argv, options, 0);
   poptSetOtherOptionHelp (pcon, "<IDL files>");
 
@@ -194,7 +188,6 @@ int main(int argc, const char *argv[])
     rinfo.enabled_passes =
       OUTPUT_COMMON | OUTPUT_HEADERS | OUTPUT_IMODULE;
 
-  rinfo.output_formatter = c_output_formatter;
   rinfo.output_language = cl_output_lang;
   rinfo.header_guard_prefix = cl_header_guard_prefix;
   rinfo.output_directory = cl_output_directory;