On Sun, Oct 9, 2011 at 12:35 PM, Douglas Gregor <[email protected]> wrote:
> I see that you're holding on to the opening source location (which is good), 
> but it doesn't look like you've simplified the interface at all. Why bother 
> to still pass in the "open" and "close" addresses in these functions:
>
> +    bool consumeOpen(SourceLocation *open) const;
> +    bool expectAndConsume(SourceLocation *open,
> +                           unsigned DiagID,
> +                           const char *Msg = "",
> +                           tok::TokenKind SkipToTok = tok::unknown) const;
> +    bool consumeClose(SourceLocation *close) const;
>
> when you could just store open/close source locations internally and add some 
> accessors
>
>        SourceRange getDelimiterRange() const;
>        SourceLocation getOpenLoc() const;
>        SourceLocation getCloseLoc() const;

Because almost every caller requires the information currently.  Out
of the 80+ calls, around 90% of them make use of the closing source
locations, and 100% make use of the open.  It seemed odd to me to
change all those usages into a function call -- then the user can call
getOpenLoc prior to calling open, getCloseLoc prior to calling close,
etc.  I thought the current approach was bit more clear, but I will
certainly change it if you disagree.

~Aaron

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to