This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/13] D: The front-end (GDC) implementation.


On 13 June 2017 at 19:29, Joseph Myers <joseph@codesourcery.com> wrote:
> As I read it, the front end has functions with names such as error, but no
> useful i18n will actually occur because the functions in d-diagnostic.cc
> format the messages with xvasprintf before passing to the common
> diagnostic code.
>

That could be changed I guess to interact with
diagnostic_report_diagnostic() directly, rather than just being a high
level wrapper around gcc error_at()/warning_at().

> But will exgettext nevertheless extract messages from the dfrontend code,
> if the functions happen to have string arguments in the same position as
> the generic diagnostic functions do?  If so, I think that should be
> disabled, to avoid putting a lot of messages in gcc.pot that won't
> actually be translated.  (If actual i18n support is desired, it should be
> shared with other users of the front end, which would mean using dgettext
> to extract translations in a different domain from the default GCC one,
> and so the messages shouldn't go in gcc.pot anyway.)
>

I would say I'm open to i18n, however upstream D probably wouldn't be.
However as it is my intention to eventually switch the dfrontend
sources to D, exgettext extracting messages would cease to be a
problem.

> In d-target.cc you have code like:
>
> +  else if (global.params.isLinux)
> +    {
> +      /* sizeof(pthread_mutex_t) for Linux.  */
> +      if (global.params.is64bit)
> +       return global.params.isLP64 ? 40 : 32;
> +      else
> +       return global.params.isLP64 ? 40 : 24;
> +    }
>
> which feels like it belongs in the config/ configuration for each target
> (as a target hook returning the required information), not in the D front
> end code.  I'm not clear what global.params.is64bit is meant to mean; it
> looks like "this is x86_64, possibly x32" in this patch.  These values
> aren't correct in general anyway; on AArch64, glibc has pthread_mutex_t of
> size 48 for LP64 and 32 for ILP32; on HPPA (only ILP32 supported for
> Linux) it's 48.
>

That is something that I have been meaning to handle better.  I was
originally thinking something along the lines of it being determined
at configure time.  Then again, it's only use is for the dfrontend to
generate a lowering of synchronized statements that looks like:

    static byte[<critsecsize>] critsec;
    _d_criticalenter(critsec.ptr);
    try { ... }
    finally {
        _d_criticalexit(critsec.ptr);
    }

Returning a size that is large enough for all would work also in the worst case.


There are a number of fields in global.params that I would prefer
removed from the shared frontend.  I when through them all with a
co-collaborator on the core dlang team during Dconf, but I wouldn't
hold my breath for it to happen.


> You have two new target macros TARGET_CPU_D_BUILTINS and
> TARGET_OS_D_BUILTINS.  You're missing any documentation for them in
> tm.texi.in.  And we prefer target hooks to macros.  So please try to
> convert them to (documented) target hooks.  (See c-family/c-target.def,
> and c_target_objs etc., for how there can be hooks that are specific to
> particular front ends.  See the comment in config/default-c.c regarding
> how to deal with a mixture of OS-dependent and architecture-dependent
> hooks.)
>

OK, thanks for the suggestion!

Iain.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]