On Tue, May 15, 2012 at 3:11 PM, Douglas Gregor <[email protected]> wrote:
> > On May 15, 2012, at 11:57 AM, David Blaikie <[email protected]> wrote: > > > [-llvm-commits since it's not too relevant to that list at this point] > > > > On Tue, May 15, 2012 at 11:24 AM, Douglas Gregor <[email protected]> > wrote: > >> > >> On May 15, 2012, at 9:31 AM, David Blaikie <[email protected]> wrote: > >> > >>> On Tue, May 15, 2012 at 9:19 AM, Douglas Gregor <[email protected]> > wrote: > >>>> > >>>> On May 6, 2012, at 7:02 PM, David Blaikie wrote: > >>>> > >>>>> This patch adds an iterator much like boost's transform_iterator > >>>>> (without some extra bells & whistles) for use in some changes I'd > like > >>>>> to make to Clang to correct some misimplemented iterators there. > >>>>> > >>>>> A few gotchas that could be improved/changed depending on opinions: > >>>>> * I may be playing a little loose with the return type of the functor > >>>>> (see the example/motivating functor, to_pointer) - the return type > >>>>> actually must be a reference, though the result_type provides the > >>>>> underlying value type, not the reference type. If this is violated > >>>>> Clang will emit a warning, but I could make it more robust with a > >>>>> compile time assertion in the TransformIterator that the result_type > >>>>> is actually a reference type, and strip that off to provide the > >>>>> value_type of the TransformIterator. > >>>> > >>>> It's much more correct for the value and reference types of the > iterator type to be, e.g., > >>>> > >>>> typedef typename Functor::result_type reference; > >>>> typedef typename remove_reference<reference>::type value_type; > >>>> > >>>>> * I realize adding pointer-yness back onto an iterator over > references > >>>>> when the underlying data structure (in the Clang patch you can see > >>>>> this situation) is a sequence of pointers may be a bit overkill - > >>>>> though the alternatives are writing different algorithms in the two > >>>>> cases where I've used TransformIterator, or having the decl_iterator > >>>>> iterate over pointers (in which case I should probably change the > >>>>> other iterators I've modified to iterate over pointers for symmetry). > >>>> > >>>> diff --git include/clang/AST/DeclBase.h include/clang/AST/DeclBase.h > >>>> index 6aef681..0a16ea5 100644 > >>>> --- include/clang/AST/DeclBase.h > >>>> +++ include/clang/AST/DeclBase.h > >>>> @@ -1175,17 +1175,18 @@ public: > >>>> Decl *Current; > >>>> > >>>> public: > >>>> - typedef Decl* value_type; > >>>> - typedef Decl* reference; > >>>> - typedef Decl* pointer; > >>>> + typedef Decl value_type; > >>>> + typedef value_type& reference; > >>>> + typedef value_type* pointer; > >>>> > >>>> Since we tend to traffic in declaration pointers, not references, > it's really beneficial to have value_type be Decl*. > >>> > >>> Fair enough - I was hoping to reduce the amount of pointers handed > >>> around, but I realize in this case especially (where the underlying > >>> sequence is a sequence of pointers) it's really an awkward fit. > >> > >> Why is reducing the number of pointers a specific goal? > > > > Evidently not something everyone finds to be a useful goal - and on > > that basis perhaps I should just leave it alone. > > > > But for myself I find functions taking references (& locals that are > > references) easier to read/reason about because it's clear that > > they're not null. Trying to decide which functions are accepting > > optional values and which ones aren't makes me pause a little - though > > perhaps more out of habit than real curiosity about whether something > > is null. If switching things to references causes pointer-habitual > > people to have the same kind of pause then that's clearly not an > > outright improvement. > > Unless we're seeing lots of problems due to the pointer-centric style, I > don't see a motivation to make this kind of change in the code base. Since there was some mention of wanting other perspectives here, I wanted to chime in with my two cents... This seems like a lot of work and a disruptive change in API patterning, and I'm really not clear what problem it's trying to solve. My suspicion is that there isn't one other than a preference for references instead of pointers. I actually don't even agree with this movement. While yes, it is nice to use simpler constructs when there is no possibility of a null pointer, references aren't strictly superior. We can't assign to them to re-point them to other entities, which is a fundamental pattern in Clang and LLVM. I don't see soft "non-null" constraints as a sufficient reason to lose this. What's more, it is trivial to add non-null constraints to pointers. In fact, dyn_cast/cast/isa all provide such constraints. We could teach Clang about [[nonnull]] for parameters and get much more than a reference gives us: - Optimizing based on non-null-ness - Checking in non-optimized builds for non-null-ness - Still able to overwrite the argument within the funciton
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
