dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.
Looks good - thanks! just a few things that could be cleaned up.
================
Comment at: include/clang/AST/Expr.h:3220
- SourceLocation getExprLoc() const LLVM_READONLY { return OpLoc; }
- SourceLocation getOperatorLoc() const { return OpLoc; }
- void setOperatorLoc(SourceLocation L) { OpLoc = L; }
+ SourceLocation getExprLoc() const { return getOperatorLoc(); }
+ SourceLocation getOperatorLoc() const { return BinaryOperatorBits.OpLoc; }
----------------
Lost LLVM_READONLY somewhere along the way? Should it be added back in?
================
Comment at: include/clang/AST/Expr.h:3256-3259
+ static bool isPtrMemOp(Opcode Opc) {
+ return Opc == BO_PtrMemD || Opc == BO_PtrMemI;
+ }
+ bool isPtrMemOp() const { return isPtrMemOp(getOpcode()); }
----------------
I probably wouldn't add a new public member function here - I take it you added
that to avoid calling getOpcode() twice? (either for performance or
textual/source tersity?)
Calling it twice probably is OK for both performance and readability - but if
you prefer otherwise, I guess you could do it with a temporary:
bool isPetrMemOp() const {
auto Opc = getOpcode();
return Opc == ... ;
}
================
Comment at: include/clang/AST/Expr.h:3369-3375
bool isFPContractableWithinStatement() const {
- return FPOptions(FPFeatures).allowFPContractWithinStatement();
+ return getFPFeatures().allowFPContractWithinStatement();
}
// Get the FENV_ACCESS status of this operator. Only meaningful for
// operations on floating point types.
+ bool isFEnvAccessOn() const { return getFPFeatures().allowFEnvAccess(); }
----------------
Could do these two (& some of the other "refactor to use a common accessor,
because the accessor is getting a bit more interesting") changes as NFC cleanup
before this - but no big deal, just mention it in case in other/future changes
that makes sense for you.
Repository:
rC Clang
https://reviews.llvm.org/D54526
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
