[cfe-commits] r103904 - in /cfe/trunk: lib/AST/ExprConstant.cpp test/CodeGenCXX/references.cpp

[Douglas Gregor]

On May 16, 2010, at 1:49 PM, Chandler Carruth wrote:

> On Sun, May 16, 2010 at 1:44 PM, Douglas Gregor <dgregor at apple.com> wrote:
> I am beginning to doubt our decision to inherit ParmVarDecl from VarDecl.
> 
> I can't tell whether that is the problem, or whether the decision to represent default arguments as initializers is the problem. Fundamentally, parameters   *are* variables within the function, so it does seem a reasonable way to represent them.

Yeah, I think breaking the ParmVarDecl -> VarDecl inheritance would create more problems than it solves. Still, it feels like I've had to add a bunch of !isa<ParmVarDecl>(Var)'s recently.

> A slight counter proposal: we re-use the storage of initializers for default arguments, but the accessor methods for initializers should filter them out, and ParmVarDecl should provide a dedicated API. Would that resolve this? Does it introduce other problems?

That would help, I think. I did a quick look through all of the getInit() calls, and the only thing that looks like it will break is PCH for ParmVarDecls with default arguments, but that's easy to fix. There are a few places where we might even fix bugs with this change :)

	- Doug

> 
> Sent from my iPhone
> 
> On May 16, 2010, at 2:32 AM, Chandler Carruth <chandlerc at gmail.com> wrote:
> 
> > Author: chandlerc
> > Date: Sun May 16 04:32:51 2010
> > New Revision: 103904
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=103904&view=rev
> > Log:
> > When constant folding reference variables with an initializer to the
> > initializer, don't fold paramters. Their initializers are just default
> > arguments which can be overridden. This fixes some spectacular regressions due
> > to more things making it into the constant folding.
> >
> > Modified:
> >    cfe/trunk/lib/AST/ExprConstant.cpp
> >    cfe/trunk/test/CodeGenCXX/references.cpp
> >
> > Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=103904&r1=103903&r2=103904&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> > +++ cfe/trunk/lib/AST/ExprConstant.cpp Sun May 16 04:32:51 2010
> > @@ -355,6 +355,10 @@
> >   } else if (VarDecl* VD = dyn_cast<VarDecl>(E->getDecl())) {
> >     if (!VD->getType()->isReferenceType())
> >       return Success(E);
> > +    // Reference parameters can refer to anything even if they have an
> > +    // "initializer" in the form of a default argument.
> > +    if (isa<ParmVarDecl>(VD))
> > +      return false;
> >     // FIXME: Check whether VD might be overridden!
> >     if (const Expr *Init = VD->getAnyInitializer())
> >       return Visit(const_cast<Expr *>(Init));
> >
> > Modified: cfe/trunk/test/CodeGenCXX/references.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/references.cpp?rev=103904&r1=103903&r2=103904&view=diff
> > ==============================================================================
> > --- cfe/trunk/test/CodeGenCXX/references.cpp (original)
> > +++ cfe/trunk/test/CodeGenCXX/references.cpp Sun May 16 04:32:51 2010
> > @@ -155,3 +155,16 @@
> > // CHECK: load
> > // CHECK: ret
> > const int &f2() { return 0; }
> > +
> > +// Don't constant fold const reference parameters with default arguments to
> > +// their default arguments.
> > +namespace N1 {
> > +  const int foo = 1;
> > +  // CHECK: @_ZN2N14test
> > +  int test(const int& arg = foo) {
> > +    // Ensure this array is on the stack where we can set values instead of
> > +    // being a global constant.
> > +    // CHECK: %args_array = alloca
> > +    const int* const args_array[] = { &arg };
> > +  }
> > +}
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20100516/be50ceac/attachment.html>