steveire 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; }
----------------
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.
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
