r188991 - Constify more uses of ASTContext&. No functional change.

John McCall rjmccall at apple.com
Fri Aug 23 11:03:14 PDT 2013


On Aug 22, 2013, at 6:25 PM, Craig Topper <craig.topper at gmail.com> wrote:
> On Thursday, August 22, 2013, John McCall wrote:
> On Aug 22, 2013, at 12:09 AM, Craig Topper <craig.topper at gmail.com> wrote:
> > Author: ctopper
> > Date: Thu Aug 22 02:09:37 2013
> > New Revision: 188991
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=188991&view=rev
> > Log:
> > Constify more uses of ASTContext&. No functional change.
> 
> Is this actually useful?  Are there interesting bugs we can catch by
> distinguishing a const vs. a non-const ASTContext?
> 
> I mean, having const AST nodes makes sense inasmuch as the AST is not
> fully immutable after construction and it’s useful to record that consumers
> are not supposed to modify the AST.  But who actually cares about having
> a const ASTContext?
> 
> Because if it’s not catching real bugs, it’s just adding a ton of noise to the
> code, and we should just say that you always have a non-const ASTContext
> the same way that you always pass around non-const llvm::Type*s.
> 
> John.
> 
> Somebody in the past went to the trouble of marking thing const and mutable within the ASTContext. I believe with Type there are no methods that can actually modify it so its const by construction.
> 
> I have no strong feelings either way. If people want me to stop ill stop.

Marking individual getters and setters as const can be useful documentation/checking even on types you don’t expect to ever see passed around as ‘const’.

I’d prefer to not see ‘const ASTContext&’ continue to propagate until someone justifies why it’s useful.

My feeling is that, all else aside, there are too many ways to get a redundant reference to the ASTContext& for this kind of const marking to be effective.

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130823/65a82c5c/attachment.html>


More information about the cfe-commits mailing list