jyu2 marked 12 inline comments as done.
jyu2 added inline comments.
================
Comment at: include/clang/AST/Stmt.h:1008
+ (*iterator_adaptor_base::I)->getStmtClass() <= lastExprConstant);
+ return *reinterpret_cast<T **>(iterator_adaptor_base::I);
}
----------------
rsmith wrote:
> Ugh. This cast violates strict aliasing, and even in the absence of strict
> aliasing won't work unless the `Stmt` base class is at offset 0 in `T`. The
> preceding assert is also wrong, as it's asserting that `*I` is an `Expr`, not
> that it's a `T`.
>
> After r352925, you can use `CastIterator<AddrLabelExpr>` instead; that should
> substantially reduce the churn in this patch.
Hi Richard,
Thanks for explain for this. And thank you so much for proving CastIterator!!
I change the code to use this.
================
Comment at: include/clang/AST/Stmt.h:2842
+
+ bool isGCCAsmGoto() const {
+ return NumLabels > 0;
----------------
rsmith wrote:
> This is already inside a class `GCCAsmStmt`; the `GCC` here seems unnecessary.
Remove GCC
================
Comment at: include/clang/AST/Stmt.h:2860
+ StringRef getLabelName(unsigned i) const;
+ Expr *getExpr(unsigned i) const;
+ using labels_iterator = ExprIterator<AddrLabelExpr>;
----------------
rsmith wrote:
> Please give this a better name; `getExpr` is pretty meaningless.
should I change to getOperandExpr?
Thanks.
================
Comment at: lib/Analysis/CFG.cpp:1474
+ appendScopeBegin(JT.block, VD, G);
+ addSuccessor(B, JT.block);
+ }
----------------
jyu2 wrote:
> efriedma wrote:
> > Please don't copy-paste code.
> :-( changed
I did this during in VisitGCCAsmStmt by not call addSuccessor when we need
backpatch the the labels.
================
Comment at: lib/Analysis/CFG.cpp:1466
+ LabelMapTy::iterator LI = LabelMap.find(E->getLabel());
+ if (LI == LabelMap.end()) continue;
+ JT = LI->second;
----------------
rsmith wrote:
> If this happens for every iteration of the loop, `VD` is used uninitialized
> after the loop.
I am totally wrong. Changed
================
Comment at: lib/Parse/ParseStmtAsm.cpp:839
+ if (Tok.isNot(tok::r_paren)) {
+ while (1) {
+ LabelDecl *LD = Actions.LookupOrCreateLabel(Tok.getIdentifierInfo(),
----------------
rsmith wrote:
> `while (true)`
Yes. Changed.
Thanks.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56571/new/
https://reviews.llvm.org/D56571
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
