On Fri, Nov 13, 2015 at 07:16:08AM -0500, David Malcolm wrote: > > > + && (TREE_CODE (TREE_TYPE (field)) == RECORD_TYPE > > > + || TREE_CODE (TREE_TYPE (field)) == UNION_TYPE)) > > > > This is RECORD_OR_UNION_TYPE_P (TREE_TYPE (field)). > > I based this code on the code in lookup_field right above it; > I copied-and-pasted that conditional, so presumably it should also be > changed in lookup_field (which has the condition twice)? > > FWIW I notice RECORD_OR_UNION_TYPE_P also covers QUAL_UNION_TYPE. > > /* Nonzero if TYPE is a record or union type. */ > #define RECORD_OR_UNION_TYPE_P(TYPE) \ > (TREE_CODE (TYPE) == RECORD_TYPE \ > || TREE_CODE (TYPE) == UNION_TYPE \ > || TREE_CODE (TYPE) == QUAL_UNION_TYPE) > > FWIW I've made the change in the attached patch (both to the new > function, and to lookup_field).
Sorry, I changed my mind. Since QUAL_UNION_TYPE is Ada-only thing and we check (RECORD_TYPE || UNION_TYPE) in a lot of places in the C FE, introducing RECORD_OR_UNION_TYPE_P everywhere would unnecessarily slow things down. I think we should have a C FE-only macro, maybe called RECORD_OR_UNION_TYPE_P that only checks for those two types, but this is something that I can deal with later on. So I think please just drop these changes for now. Sorry again. > > > + const bool debug = false; > > > + > > > + if (debug) > > > + { > > > + printf ("s: \"%s\" (len_s=%i)\n", s, len_s); > > > + printf ("t: \"%s\" (len_t=%i)\n", t, len_t); > > > + } > > > > Did you leave this debug stuff here intentionally? > > I find it useful, but I believe it's against our policy, so I've deleted > it in the attached patch. Probably. But you could surely have a separate DEBUG_FUNCTION that can be called from gdb. > > > + /* Build the rest of the row by considering neighbours to > > > + the north, west and northwest. */ > > > + for (int j = 0; j < len_s; j++) > > > + { > > > + edit_distance_t cost = (s[j] == t[i] ? 0 : 1); > > > + edit_distance_t deletion = v1[j] + 1; > > > + edit_distance_t insertion = v0[j + 1] + 1; > > > > The formatting doesn't look right here. > > It's correct; it's "diff" inserting two spaces before a tab combined > with our mixed spaces+tab convention: the "for" is at column 6 (6 > spaces), whereas the other lines are at column 8 (1 tab), which looks > weird in a diff. Sorry, what I had in mind were the spaces after "deletion" and "insertion" before "=". Not a big deal, of course. > Patch attached; only tested lightly so far (compiles, and passes > spellcheck subset of tests). > > OK for trunk if it passes bootstrap®rtest? Ok modulo the RECORD_OR_UNION_TYPE_P changes, thanks. Marek