On 6 December 2016 at 19:39, Vitaly Buka <vitalyb...@google.com> wrote:
> Hi Alex, > > > > On Tue, Dec 6, 2016 at 11:14 AM Alex L <arpha...@gmail.com> wrote: > > Hi Vitaly, > > I noticed that you posted this patch up for review at > https://reviews.llvm.org/D27422, but then committed it instantly without > waiting for approval (and you did the same for r288685 as well). Is there > any particular reason why you did this? I think that you should've waited > for approval before committing. > > > We had broken build bots, so seem like this trivial change is better than > reverting patches. > No.3 of http://llvm.org/docs/DeveloperPolicy.html#code-reviews allows > after commit review for changes like this. > Thanks for the clarification. Yes, the developer policy states that you can commit small changes and get them reviewed after committing, so a commit like this would've been perfect for a post-commit review. However, it also says that "Specifically, once a patch is sent out for review, it needs an explicit “looks good” before it is submitted". Please avoid submitting patches for review if you know that you will commit them without waiting for approval in the future. > > > This patch is pretty small, and so it looks to me like it could have been > reviewed after it was committed, but patches that get post-commit reviews > shouldn't get explicit review requests like this one did. > > > Sorry, probably I did the same few time before. I can't find exact details > in the policy, but I assumed that was a reasonable approach. > So what is the process for after commit review? > People usually read the commit logs and check which commits need post-commit reviews, and then look at them. There's no need for an explicit review request for commits like this. > > > > Alex > > On 5 December 2016 at 19:25, Vitaly Buka via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > > Author: vitalybuka > Date: Mon Dec 5 13:25:00 2016 > New Revision: 288689 > > URL: http://llvm.org/viewvc/llvm-project?rev=288689&view=rev > Log: > Fix stack-use-after-scope in CheckExplicitlyDefaultedMemberExceptionSpec > > Summary: > Similar to r288685. > getExceptionSpec returned structure with pointers to temporarily object > created > by computeImplicitExceptionSpec. > > Reviewers: rsmith > > Subscribers: aizatsky, cfe-commits > > Differential Revision: https://reviews.llvm.org/D27422 > > Modified: > cfe/trunk/lib/Sema/SemaDeclCXX.cpp > > Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/ > SemaDeclCXX.cpp?rev=288689&r1=288688&r2=288689&view=diff > ============================================================ > ================== > --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Dec 5 13:25:00 2016 > @@ -6299,8 +6299,8 @@ void Sema::CheckExplicitlyDefaultedMembe > CallingConv CC = Context.getDefaultCallingConvention(/* > IsVariadic=*/false, > > /*IsCXXMethod=*/true); > FunctionProtoType::ExtProtoInfo EPI(CC); > - EPI.ExceptionSpec = computeImplicitExceptionSpec(*this, > MD->getLocation(), MD) > - .getExceptionSpec(); > + auto IES = computeImplicitExceptionSpec(*this, MD->getLocation(), MD); > + EPI.ExceptionSpec = IES.getExceptionSpec(); > const FunctionProtoType *ImplicitType = cast<FunctionProtoType>( > Context.getFunctionType(Context.VoidTy, None, EPI)); > > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
