riccibruno marked 3 inline comments as done.
riccibruno added inline comments.
================
Comment at: include/clang/AST/Expr.h:5095
- SourceLocation getBeginLoc() const LLVM_READONLY { return GenericLoc; }
- SourceLocation getEndLoc() const LLVM_READONLY { return RParenLoc; }
----------------
steveire wrote:
> riccibruno wrote:
> > steveire wrote:
> > > Why remove the `LLVM_READONLY` from this class instead of from everywhere
> > > in the file it is not needed? (in a separate commit, obviously).
> > `LLVM_READONLY` is a macro for `__attribute__((pure))` (when supported). It
> > is useful in some cases to give a hint to the optimizer about the behavior
> > of a function. However in this case the definition of the function is
> > visible in all translation units using this function. Therefore the
> > optimizer can very well see on its own what this function is doing.
> >
> > It would be perfectly possible to inspect all uses of `LLVM_READONLY` and
> > only keep what is needed, but this is such a small issue that nobody
> > bothered to do it until now.
> My point was again about commit hygiene.
>
> You could make a commit changing this without further review, leaving this
> review cleaner. You could do the same with the moved friend decl. Commits
> which only do one thing are easier to review.
Ah I see. Yes I can do this.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57104/new/
https://reviews.llvm.org/D57104
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
