Skip to content
  • Steinar H. Gunderson's avatar
    bca1bf1f
    WL #13990: Unify the two field lists in SELECT_LEX · bca1bf1f
    Steinar H. Gunderson authored
    In SELECT_LEX, unify fields_list (visible fields) and all_fields
    (visible _and_ invisible fields); before this patch, they were
    overlapping linked lists (except in a few cases where they were
    not, undocumented). This is necessary but not sufficient to be
    able to have visible and invisible items in arbitrary order;
    in order to get that, we will also need to remove most cases of
    thd->change_item_tree(), and a few other things, detailed in
    comments. However, this is a good cleanup in itself, and enables
    us to use e.g. range-based for loops in a number of places.
    
    The new data structure is an std::deque of Item *, which has
    much less confusing semantics, is more cache-efficient and also
    works without constant pointer chasing. (Ideally, we'd move
    towards a Mem_root_array, but cannot do it before the aforementioned
    changes.) This type change propagates far into the code, but is mostly
    localized to the optimizer. (It removes about 80% of all instances of
    List<Item>, which is a good step on the way to removing List<T> entirely.)
    Item has received a boolean that signifies whether it is hidden or not,
    and many loops that iterated over fields_list now check this flag and
    skip hidden ones.
    
    As List used empty() to clear a list, but std::deque<T> uses that
    to return whether the list is empty or not (so fields.empty() is
    a no-op with no compiler warnings), there was a huge risk of
    introducing wrong code during merges and development, ie., code
    that would call fields.empty(), causing very hard-to-find
    bugs. For this reason, List<T>::empty() has been renamed to
    List<T>::clear(). is_empty() has _not_ been renamed to empty(),
    so that we get compile errors on such merges and can catch them
    early.
    
    Many places that sent a non-const reference to List<Item> have
    been changed to either a const reference or a pointer, as the
    style guide mandates (most of this code predates the style guide).
    This makes it clear whether the fields list is to be modified
    or not.
    
    Change-Id: I520f680416f5bf2dcdd61d002eea02878cb1dc10
    bca1bf1f
    WL #13990: Unify the two field lists in SELECT_LEX
    Steinar H. Gunderson authored
    In SELECT_LEX, unify fields_list (visible fields) and all_fields
    (visible _and_ invisible fields); before this patch, they were
    overlapping linked lists (except in a few cases where they were
    not, undocumented). This is necessary but not sufficient to be
    able to have visible and invisible items in arbitrary order;
    in order to get that, we will also need to remove most cases of
    thd->change_item_tree(), and a few other things, detailed in
    comments. However, this is a good cleanup in itself, and enables
    us to use e.g. range-based for loops in a number of places.
    
    The new data structure is an std::deque of Item *, which has
    much less confusing semantics, is more cache-efficient and also
    works without constant pointer chasing. (Ideally, we'd move
    towards a Mem_root_array, but cannot do it before the aforementioned
    changes.) This type change propagates far into the code, but is mostly
    localized to the optimizer. (It removes about 80% of all instances of
    List<Item>, which is a good step on the way to removing List<T> entirely.)
    Item has received a boolean that signifies whether it is hidden or not,
    and many loops that iterated over fields_list now check this flag and
    skip hidden ones.
    
    As List used empty() to clear a list, but std::deque<T> uses that
    to return whether the list is empty or not (so fields.empty() is
    a no-op with no compiler warnings), there was a huge risk of
    introducing wrong code during merges and development, ie., code
    that would call fields.empty(), causing very hard-to-find
    bugs. For this reason, List<T>::empty() has been renamed to
    List<T>::clear(). is_empty() has _not_ been renamed to empty(),
    so that we get compile errors on such merges and can catch them
    early.
    
    Many places that sent a non-const reference to List<Item> have
    been changed to either a const reference or a pointer, as the
    style guide mandates (most of this code predates the style guide).
    This makes it clear whether the fields list is to be modified
    or not.
    
    Change-Id: I520f680416f5bf2dcdd61d002eea02878cb1dc10
Loading