On Fri, Sep 03, 2004 at 03:01:01PM -0500, Anshu Dasgupta wrote:> ><snip> > >for (BasicBlock::iterator I = Dest->begin(); PHINode *PN = > >dyn_cast<PHINode>(I); ++I) > > visitPHINode(*PN); > ><snip> > > > >build_vc71\lib\Transforms\Scalar\SCCP.cpp(202) : error C2275: > >'llvm::PHINode' : illegal use of this type as an expression > > > >but I think is a NO-NO, so suggestions? > > Since it's fussy about a declaration in the for construct, perhaps > something like this might work: > > PHINode *PN; > for (BasicBlock::iterator I = Dest->begin(); (PN = > dyn_cast<PHINode>(I)); ++I) > ...I think what you're arguing are really the same thing. The for loop uses a dyn_cast<> which returns non-zero on success. Paolo converted dyn_cast<> to isa<> which returns true precisely when dyn_cast<> would return non-zero (which is what the for-loop actually does). So I would say that Paolo's solution is just as valid, and possibly cleaner, since isa<> returns a bool, while dyn_cast<> returns a pointer. -- Misha Brukman :: http://misha.brukman.net :: http://llvm.cs.uiuc.edu
I can confirm that both are compiled properly: A) PHINode *PN; for (BasicBlock::iterator I = H->begin(); PN = dyn_cast<PHINode>(I); I++) .... B) for (BasicBlock::iterator I = H->begin; isa<PHINode>(I); I++) { PHINode *PN = cast<PHINode(I); .... } I'll make a patch for whatever solution do you prefer (this problem is a showstopper for more than a dozen files...) --- Paolo Invernizzi Post Scriptum.... please remember I have no access to regression tests (still <g>) so please keep all my submission with "grano salis" On Sep 3, 2004, at 10:07 PM, Misha Brukman wrote:> On Fri, Sep 03, 2004 at 03:01:01PM -0500, Anshu Dasgupta wrote: >>> <snip> >>> for (BasicBlock::iterator I = Dest->begin(); PHINode *PN >>> dyn_cast<PHINode>(I); ++I) >>> visitPHINode(*PN); >>> <snip> >>> >>> build_vc71\lib\Transforms\Scalar\SCCP.cpp(202) : error C2275: >>> 'llvm::PHINode' : illegal use of this type as an expression >>> >>> but I think is a NO-NO, so suggestions? >> >> Since it's fussy about a declaration in the for construct, perhaps >> something like this might work: >> >> PHINode *PN; >> for (BasicBlock::iterator I = Dest->begin(); (PN >> dyn_cast<PHINode>(I)); ++I) >> ... > > I think what you're arguing are really the same thing. > The for loop uses a dyn_cast<> which returns non-zero on success. > Paolo converted dyn_cast<> to isa<> which returns true precisely when > dyn_cast<> would return non-zero (which is what the for-loop actually > does). So I would say that Paolo's solution is just as valid, and > possibly cleaner, since isa<> returns a bool, while dyn_cast<> returns > a > pointer. > > -- > Misha Brukman :: http://misha.brukman.net :: http://llvm.cs.uiuc.edu > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://mail.cs.uiuc.edu/mailman/listinfo/llvmdev
On Fri, 3 Sep 2004, Paolo Invernizzi wrote:> I can confirm that both are compiled properly:Ok.> for (BasicBlock::iterator I = H->begin; isa<PHINode>(I); I++) { > PHINode *PN = cast<PHINode(I); > .... > } > > I'll make a patch for whatever solution do you prefer (this problem is > a showstopper for more than a dozen files...)I prefer this option (it reduces the scope of the PN variable). -Chris> > Post Scriptum.... please remember I have no access to regression tests > (still <g>) so please keep all my submission with "grano salis" > > > On Sep 3, 2004, at 10:07 PM, Misha Brukman wrote: > > > On Fri, Sep 03, 2004 at 03:01:01PM -0500, Anshu Dasgupta wrote: > >>> <snip> > >>> for (BasicBlock::iterator I = Dest->begin(); PHINode *PN > >>> dyn_cast<PHINode>(I); ++I) > >>> visitPHINode(*PN); > >>> <snip> > >>> > >>> build_vc71\lib\Transforms\Scalar\SCCP.cpp(202) : error C2275: > >>> 'llvm::PHINode' : illegal use of this type as an expression > >>> > >>> but I think is a NO-NO, so suggestions? > >> > >> Since it's fussy about a declaration in the for construct, perhaps > >> something like this might work: > >> > >> PHINode *PN; > >> for (BasicBlock::iterator I = Dest->begin(); (PN > >> dyn_cast<PHINode>(I)); ++I) > >> ... > > > > I think what you're arguing are really the same thing. > > The for loop uses a dyn_cast<> which returns non-zero on success. > > Paolo converted dyn_cast<> to isa<> which returns true precisely when > > dyn_cast<> would return non-zero (which is what the for-loop actually > > does). So I would say that Paolo's solution is just as valid, and > > possibly cleaner, since isa<> returns a bool, while dyn_cast<> returns > > a > > pointer. > > > > -- > > Misha Brukman :: http://misha.brukman.net :: http://llvm.cs.uiuc.edu > > > > _______________________________________________ > > LLVM Developers mailing list > > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > > http://mail.cs.uiuc.edu/mailman/listinfo/llvmdev > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://mail.cs.uiuc.edu/mailman/listinfo/llvmdev >-Chris -- http://llvm.org/ http://nondot.org/sabre/