This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 2/13] D: The front-end (GDC) implementation.
- From: Iain Buclaw <ibuclaw at gdcproject dot org>
- To: Joseph Myers <joseph at codesourcery dot com>
- Cc: gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 14 Jun 2017 00:41:26 +0200
- Subject: Re: [PATCH 2/13] D: The front-end (GDC) implementation.
- Authentication-results: sourceware.org; auth=none
- References: <CABOHX+eUKoD4VOdsmaQW+BC2fwt1v0r_OaAABz_TjjdJj7OhDQ@mail.gmail.com> <alpine.DEB.2.20.1706131712510.11434@digraph.polyomino.org.uk>
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.