From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14140 invoked by alias); 8 Jun 2013 13:35:45 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 14130 invoked by uid 89); 8 Jun 2013 13:35:45 -0000 X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,SPF_PASS autolearn=ham version=3.3.1 Received: from mail-vc0-f180.google.com (HELO mail-vc0-f180.google.com) (209.85.220.180) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Sat, 08 Jun 2013 13:35:06 +0000 Received: by mail-vc0-f180.google.com with SMTP id gf11so1373001vcb.25 for ; Sat, 08 Jun 2013 06:35:05 -0700 (PDT) X-Received: by 10.52.93.46 with SMTP id cr14mr1236420vdb.132.1370698505121; Sat, 08 Jun 2013 06:35:05 -0700 (PDT) MIME-Version: 1.0 Received: by 10.52.188.201 with HTTP; Sat, 8 Jun 2013 06:34:44 -0700 (PDT) In-Reply-To: <51B0F122.6020301@redhat.com> References: <51B0B0ED.5090508@redhat.com> <51B0F122.6020301@redhat.com> From: Andrew Sutton Date: Sat, 08 Jun 2013 13:35:00 -0000 Message-ID: Subject: Re: [c++-concepts] code review To: Jason Merrill Cc: Gabriel Dos Reis , gcc-patches@gcc.gnu.org Content-Type: text/plain; charset=ISO-8859-1 X-SW-Source: 2013-06/txt/msg00446.txt.bz2 >> +C++ ObjC++ Var(flag_concepts, true) > > This line declares flag_concepts implicitly. Ah... I see. Fixed. >> That's the long and short of it. Gaby suggested writing constraints >> here so that, for any instantiation, you would have easy access to the >> constraints for that declaration. > > I'm not sure why you would care about the constraints for a particular > instantiation; constraints only apply to the template, right? In concepts lite, yes. Moving forward... I'm less certain. In the context of separate checking, instantiated requirements may carry lookup information into the current instantiation. But that's just speculation. I think I previously put constraint_info in lang_decl_min, right next to template_info no less. It was easy to manage there, and initialized as part of build_template_decl. But this obviously doesn't work for partial specializations unless they get template_decls. I'd be okay committing the current design with the caveat that it may need to be rewritten in the not-so-distant future. I've already written the other other way two or three times, so I'm familiar with those changes. >> branch_goal queues a copy of the current sub-goal, returning a >> reference to that new branch. The insertion of the operands are done >> on different sub-goals, giving the expected results. > > Right, I suppose I should have paid more attention to "This does not update > the current goal"... On the topic of logic.cc, I'm plan on rewriting this module to use a set instead of lists. There will be some pretty substantial changes to the internal interfaces. Would it be reasonable to commit this now (since it works correctly), and plan to rewrite it in the near future? > template > tree > extract_goals (proof_state& s) > ... > return extract_goals(s); > > but I suppose STL style is OK, too. Huh. I didn't realize that could be inlined. Neat. >> I was trying to write the parsing code a little more modularly so I >> could keep my parse functions as small as possible. I use the facility >> more heavily in the requires/validexpr code that's not included here. > > > Hmm, to me it seems more modular to keep all of the code for handling e.g. > "requires" in its own function rather than needing two different places to > know how a requires clause starts. I think I see where the problem is. cp_parser_requires_clause is parsed non-optionally in a requires expression, but that's not included in the patch. I factored out the explicit parsing (hence the assertion) from the optional parsing. I could remove the assertion. There's no path to that function that does not first check that 'requires' has been found. >>> Why don't you use 'release' and conjoin_requirements here? >> >> >> Because there is no template parameter list that can provide >> additional requirements in this declaration. > > > OK, please add a comment to that effect. On second thought, the absence of release() may actually have been a bug. Comment added. >>> The comment sounds more like tsubst_template_parms than >>> coerce_template_parms. >> >> >> It might be... I'll have to look. What I actually want to get is the >> set of actual arguments that will be substituted for template >> parameters given an initial set of arguments (lookup default >> arguments, generate pack arguments, etc). > > > Right, I think coerce_template_parms has the effect you want, I just don't > think of it as doing substitution, so the comment and name could use a > tweak. If the function doesn't go away, that is. Still looking at this. What is the main entry point to overload resolution? Andrew