This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: gengtype improvements for plugins, completed! patch 3/N [inputfile]
On Fri, 10 Sep 2010 05:36:22 +0300
Laurynas Biveinis <laurynas.biveinis@gmail.com> wrote:
>
> - const char *file, int line)
> + input_file* inpf, int line)
>
> const input_file * ?
>
> -/* For F a filename, return the relative path to F from $(srcdir) if the
> +/* For F an input_file, return the relative path to F from $(srcdir) if the
> latter is a prefix in F, or the real basename of F otherwise. */
>
> static const char *
> -get_file_basename (const char *f)
> +get_file_basename (const input_file *inpf)
> {
>
> Comment out of sync with the function signature (INPF not F)
>
> -get_output_file_with_visibility (const char *input_file)
> +get_output_file_with_visibility (input_file* inpf)
>
> const
>
> -get_output_file_name (const char *input_file)
> +get_output_file_name (input_file *inpf)
>
Thanks for the comments, I am replying here only to one: having all
input_file* be const.
In principle you are right, in the sense that all the functions you
mentioned do not conceptually modify the input_file structure, with
however one significant exception, its inpoutf field which is caching
the result of get_ouput_file_with_visibility. In C++ parlance that
field would probably be declared as mutable, since it can change even
in functions dealing with const input_file* but gengtype is not in C++!
All these functions may indirectly call
get_output_file_with_visibility. See the next patch 4/N [files_rules]
which I did not yet send in its "completed!" form, but which I first
sent in http://gcc.gnu.org/ml/gcc-patches/2010-08/msg02065.html and
which you commented in
http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00419.html etc.
And the new get_output_file_with_visibility will indeed use and update
the inpoutf
So I see several ways of dealing with that:
A. Use const input_file* in every function which conceptually don't
modify input_file-s, but then adding some CONST_CAST at every call to
get_output_file_with_visibility.
B. Declare every function with const input_file* including even
get_output_file_with_visibility which will have to CONST_CAST its
argument to update its inpoutf field. But that function indeed and
importantly updates the inpoutf field of input_file and honestly is not
dealing with its input_file* as a constant argument.
C. remove every const for input_file* functions, since they all
indirectly can modifiy its inpoutf field thru
get_output_file_with_visibility.
What do people think? I believe that I tried to implement solution A,
but my personal preference go to C, which would make a bigger patch.
[I tried nearly systematically to make the patches shorter than longer].
I would like comments on this (in particular by reviewers & by
Laurynas).
In general, I tend to dislike the const keyword in C code, except when
it indicates that the data is in read-only segment and would SEGV when
modified. However, I do admit it permit better diagnostics from the
compiler.
Cheers.
--
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***