From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf32.google.com (mail-qv1-xf32.google.com [IPv6:2607:f8b0:4864:20::f32]) by sourceware.org (Postfix) with ESMTPS id 233713858C20 for ; Tue, 11 Oct 2022 11:42:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 233713858C20 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=kitware.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kitware.com Received: by mail-qv1-xf32.google.com with SMTP id l19so8745943qvu.4 for ; Tue, 11 Oct 2022 04:42:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kitware.com; s=google; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=x0xurbxnLsOhlKobpWuT3eC9Eno5oC0V4RNcMxIJg/Q=; b=KtmnRLsMgNkhan5JhqgxOW42V0OKdSTjZDXw+X7IMP02ckD7zKNV5/7CcIeYvJD9B1 lkroN5Db2mRQJCimNG6hIgc+tUBcS+HeMzVaNpdFMV+BWWkwqHzVWmdohiP4NbZP2ZvA rlAf4xg9wdI5MoomWeoE+Qh7HayV3m8/vkCxw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=x0xurbxnLsOhlKobpWuT3eC9Eno5oC0V4RNcMxIJg/Q=; b=JaeT1xx4HkTCDD95glWBHYVWXz1KUtihkkYHl/B9KB5pCBr3xJf3eFB/NtfKGrF8d0 lx/0HBdsR6L49/Q2q8uZz82EEMZdDQE6cKY8d+4U6Jj0i24moC50BKbBtVNoW8UItHeX 51ldqn/hMlgujjiUt7WkKdhGEtPJF0+UxOOnhd27O/8CbmNF6kezRKnNFgyQkZg+TnG2 +MhPDxaDeQzX3+hPhrIzzeq3XpJAvwdKSxpHMklrQdDBaYDbyTyqlAdfxlTzWwsVYsge KzzC9AzLxt1nw1Kk9r87D5P4z0zVqNBlkh5KEgLM44ZxzC8EJgFuCfUU5l6G7RlIf+yg M1Dg== X-Gm-Message-State: ACrzQf2W0F8AgMtDMNl07UFyF/db37OqO9VuhELPqS6tWJHDJs3QdL7D YdsxiVZ122/gZb3XO/o51YpsOQ== X-Google-Smtp-Source: AMsMyM7F6nvcj3JoooxKf4sNgF+xNsTrTC7dUngnyAmw/mlW1Nk7ZuHlCaLdWmA6Q2Pa7kgxav11BA== X-Received: by 2002:a05:6214:f04:b0:4b1:cb3b:79bd with SMTP id gw4-20020a0562140f0400b004b1cb3b79bdmr18736846qvb.22.1665488563543; Tue, 11 Oct 2022 04:42:43 -0700 (PDT) Received: from localhost (cpe-142-105-146-128.nycap.res.rr.com. [142.105.146.128]) by smtp.gmail.com with ESMTPSA id w27-20020a05620a0e9b00b006ee7923c187sm2031256qkm.42.2022.10.11.04.42.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 11 Oct 2022 04:42:42 -0700 (PDT) Date: Tue, 11 Oct 2022 07:42:41 -0400 From: Ben Boeckel To: Jason Merrill Cc: Ben Boeckel , gcc-patches@gcc.gnu.org, nathan@acm.org, fortran@gcc.gnu.org, gcc@gcc.gnu.org, brad.king@kitware.com Subject: Re: [PATCH RESEND 1/1] p1689r5: initial support Message-ID: References: <20221004151200.1275636-1-ben.boeckel@kitware.com> <20221004151200.1275636-2-ben.boeckel@kitware.com> <78b88b1d-b328-a140-3a27-d33a3d96f3b9@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <78b88b1d-b328-a140-3a27-d33a3d96f3b9@redhat.com> User-Agent: Mutt/2.2.7 (2022-08-07) X-Spam-Status: No, score=-10.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, Oct 10, 2022 at 17:04:09 -0400, Jason Merrill wrote: > On 10/4/22 11:12, Ben Boeckel wrote: > > This patch implements support for [P1689R5][] to communicate to a build > > system the C++20 module dependencies to build systems so that they may > > build `.gcm` files in the proper order. > > Thanks! > > > Support is communicated through the following three new flags: > > > > - `-fdeps-format=` specifies the format for the output. Currently named > > `p1689r5`. > > > > - `-fdeps-file=` specifies the path to the file to write the format to. > > Do you expect users to want to emit Makefile (-MM) and P1689 > dependencies from the same compilation? Yes, the build system wants to know what files affect scanning as well (e.g., `#include ` is still possible and if it changes, this operation should be performed again. The `-M` flags do this quite nicely already :) . > > - `-fdep-output=` specifies the `.o` that will be written for the TU > > that is scanned. This is required so that the build system can > > correlate the dependency output with the actual compilation that will > > occur. > > The dependency machinery already needs to be able to figure out the name > of the output file, can't we reuse that instead of specifying it on the > command line? This is how it determines the output of the compilation. Because it is piggy-backing on the `-E` flag, the `-o` flag specifies the output of the preprocessed source (purely a side-effect right now). > > diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h > > index 2db1e9cbdfb..90787230a9e 100644 > > --- a/libcpp/include/cpplib.h > > +++ b/libcpp/include/cpplib.h > > @@ -298,6 +298,9 @@ typedef CPPCHAR_SIGNED_T cppchar_signed_t; > > /* Style of header dependencies to generate. */ > > enum cpp_deps_style { DEPS_NONE = 0, DEPS_USER, DEPS_SYSTEM }; > > > > +/* Format of header dependencies to generate. */ > > +enum cpp_deps_format { DEPS_FMT_NONE = 0, DEPS_FMT_P1689R5 }; > > Why not add this to cpp_deps_style? It is orthogonal. The `-M` flags and `-fdeps-*` flags are similar, but `-fdeps-*` dependencies are fundamentally different than `-M` dependencies. The comment does need updated though. `-M` is about discovered dependencies: those that you find out while doing work. `-fdep-*` is about ordering dependencies: extracting information from file content in order to even order future work around. It stems from the loss of embarassing parallelism when building C++20 code that uses `import`. It's isomorphic to the Fortran module compilation ordering problem. > > @@ -387,7 +410,7 @@ make_write_vec (const mkdeps::vec &vec, FILE *fp, > > .PHONY targets for all the dependencies too. */ > > > > static void > > -make_write (const cpp_reader *pfile, FILE *fp, unsigned int colmax) > > +make_write (const cpp_reader *pfile, FILE *fp, unsigned int colmax, int extra) > > Instead of adding the "extra" parameter... Hmm. Not sure why I had named this so poorly. Maybe this comes from my initial stab at this functionality in 2019 (the format has been hammered out in ISO C++'s SG15 since then). > > { > > const mkdeps *d = pfile->deps; > > > > @@ -398,7 +421,7 @@ make_write (const cpp_reader *pfile, FILE *fp, unsigned int colmax) > > if (d->deps.size ()) > > { > > column = make_write_vec (d->targets, fp, 0, colmax, d->quote_lwm); > > - if (CPP_OPTION (pfile, deps.modules) && d->cmi_name) > > + if (extra && CPP_OPTION (pfile, deps.modules) && d->cmi_name) > > column = make_write_name (d->cmi_name, fp, column, colmax); > > fputs (":", fp); > > column++; > > @@ -412,7 +435,7 @@ make_write (const cpp_reader *pfile, FILE *fp, unsigned int colmax) > > if (!CPP_OPTION (pfile, deps.modules)) > > return; > > ...how about checking CPP_OPTION for p1689r5 mode here? I'll take a look at this. > > > > - if (d->modules.size ()) > > + if (extra && d->modules.size ()) > > { > > column = make_write_vec (d->targets, fp, 0, colmax, d->quote_lwm); > > if (d->cmi_name) > > @@ -423,7 +446,7 @@ make_write (const cpp_reader *pfile, FILE *fp, unsigned int colmax) > > fputs ("\n", fp); > > } > > > > - if (d->module_name) > > + if (extra && d->module_name) > > { > > if (d->cmi_name) > > { > > @@ -455,7 +478,7 @@ make_write (const cpp_reader *pfile, FILE *fp, unsigned int colmax) > > } > > } > > > > - if (d->modules.size ()) > > + if (extra && d->modules.size ()) > > { > > column = fprintf (fp, "CXX_IMPORTS +="); > > make_write_vec (d->modules, fp, column, colmax, 0, ".c++m"); > > @@ -468,9 +491,203 @@ make_write (const cpp_reader *pfile, FILE *fp, unsigned int colmax) > > /* Really we should be opening fp here. */ > > > > void > > -deps_write (const cpp_reader *pfile, FILE *fp, unsigned int colmax) > > +deps_write (const struct cpp_reader *pfile, FILE *fp, unsigned int colmax, > > + int extra) > > +{ > > + make_write (pfile, fp, colmax, extra); > > +} > > + > > +static bool > > +is_utf8 (const char *name) > > Can we share utf8 parsing code with decode_utf8_char in pretty-print.cc? I can look at factoring that out. I'll have to decode its logic to see how much overlap there is. Thanks, --Ben