[PATCH] D56676: Introduce forward definitions

Thomas Rodgers rodgert at twrodgers.com
Mon Jan 14 20:39:37 PST 2019


I pulled this patch, I need to adjust the declarations because you added
the execution policy to the signatures. I have a separate experimental fork
which does the same, but it is not part of what was submitted for GCC9.

I will resubmit this patch after adjusting the signatures. There will be a
follow on patch that actually includes these forward definitions
appropriately.

As for rationalising file names, I don’t have any objections, but I also
don’t have any suggestions other than libstdc++ use ‘_fwd.h’ as a naming
convention for forward declarations and we could certainly rename the
‘glue_Xxx.h’ to something else to be more consistent.

Having said all of that, I have a very narrow window to still make the GCC9
release so I will not be entertaining much churn in the source structure
beyond this merge for that deliverable.

On Mon, Jan 14, 2019 at 12:45 PM Alexey Kukanov via Phabricator <
reviews at reviews.llvm.org> wrote:

> akukanov added a comment.
>
> As I understand, this patch adds forward declarations for all internal
> functions, similarly to "defs" files having declarations for all functions
> in `namespace std`. Perhaps the file names should be aligned with the
> existing files, e.g. `glue_algorithm_internal_defs.h` etc.
>
> Also the new files are not included anywhere. Will there be a separate
> patch that uses these files, or they are intended solely for integration
> into C++ libraries (or even solely libstdc++)? If the latter, perhaps a
> usage example could be documented somewhere - in the comments or maybe in a
> separate readme-like file.
>
>
>
> ================
> Comment at: include/pstl/internal/parallel_backend_tbb_fwd.h:11
> +
> +#ifndef __PSTL_parallel_backend_tbb_fwd_H
> +#define __PSTL_parallel_backend_tbb_fwd_H
> ----------------
> The backend API was supposed to be implementation independent. Maybe there
> should be no "tbb" in the file name?
>
>
> ================
> Comment at: include/pstl/internal/parallel_backend_tbb_fwd.h:47-50
> +struct backend_impl_t
> +{
> +    static cancel_execution();
> +};
> ----------------
> Currently, `cancel_execution()` is a standalone function in namespace
> `par_backend`. Is this mismatch intentional or accidental?
>
>
> Repository:
>   rPSTL pstl
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D56676/new/
>
> https://reviews.llvm.org/D56676
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20190114/a057f29a/attachment.html>


More information about the libcxx-commits mailing list