| Message ID | CAFiYyc2pXh+sChUP91Yn3e9EF87zR7KURh6EiF7YdcThZ-2gBA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Hi, On Thu, 20 Nov 2014, Richard Biener wrote: > > I don't think this API will make the non-C++-fans happier; I think the > > objection to the work I just merged is that it's adding more C++ than > > those people are comfortable with. > > How so? It's already super-ugly in those views. We decided to get C++. > Now we have it. And? Nobody says we can't have nice looking code even with C++. > Now please make it AT LEAST CONSISTENT. True. > > I suspect that any API which requires the of < > characters within the > > implementation of a gimple pass to mean a template is going to give > > those less happy with C++ a visceral "ugh" reaction. I wonder if > > there's a way to spell these things that's concise and which doesn't > > involve <> ? > > Only if you drop as_a/is_a/dyn_cast everywhere. Oh god, yes. Please! IMHO they don't accomplish much, but make code harder to visually parse. They don't accomplish much because you have to write the snippets that check validity of conversions anyway, so they can just as well be written as proper methods or global functions, or even just conversion operators. Nothing forces us to implement these snippets as noisy template specializations like: template <> template <> inline bool is_a_helper <cgraph_node *>::test (symtab_node *p) { return p->type == SYMTAB_FUNCTION; } instead of the more mundane means. And once you have those snippets as normal functions, you can just as well call them like they are functions, making the using side of those conversion also look nicer. Ciao, Michael.
On Thu, Nov 20, 2014 at 4:34 PM, Michael Matz <matz@suse.de> wrote: > Hi, > > On Thu, 20 Nov 2014, Richard Biener wrote: > >> > I don't think this API will make the non-C++-fans happier; I think the >> > objection to the work I just merged is that it's adding more C++ than >> > those people are comfortable with. >> >> How so? It's already super-ugly in those views. We decided to get C++. >> Now we have it. > > And? Nobody says we can't have nice looking code even with C++. > >> Now please make it AT LEAST CONSISTENT. > > True. > >> > I suspect that any API which requires the of < > characters within the >> > implementation of a gimple pass to mean a template is going to give >> > those less happy with C++ a visceral "ugh" reaction. I wonder if >> > there's a way to spell these things that's concise and which doesn't >> > involve <> ? >> >> Only if you drop as_a/is_a/dyn_cast everywhere. > > Oh god, yes. Please! IMHO they don't accomplish much, but make code > harder to visually parse. They don't accomplish much because you > have to write the snippets that check validity of conversions anyway, so > they can just as well be written as proper methods or global functions, or > even just conversion operators. Nothing forces us to implement these > snippets as noisy template specializations like: > > template <> > template <> > inline bool > is_a_helper <cgraph_node *>::test (symtab_node *p) > { > return p->type == SYMTAB_FUNCTION; > } > > instead of the more mundane means. And once you have those snippets as > normal functions, you can just as well call them like they are functions, > making the using side of those conversion also look nicer. True. I don't remember exactly but exclusively using member functions wasn't in the list of proposals that ended up with us doing as_a/is_a as it is done now. Can unions / PODs have such member functions? Just thinking about a reason why it wasn't proposed. Btw, I don't see as_a/is_a/dyn_cast as super-ugly - it's actually a perfectly fine C++-way of doing RTTI. I also guess that it requires less code in the actual implementation as we share one helper for all three operations. Of course we could macroize the member function implementation in some clever way .... That said - we have a substantial amount of code using as_a/is_a/dyn_cast and I don't think it's appropriate at this point to change all of it to a different mechanism. Proposals with example patches are of course welcome, but beware that if you want to succeed here then as-a.h needs to go ;) Thanks, Richard. > > Ciao, > Michael.
diff --git a/gcc/asan.c b/gcc/asan.c index be28ede..d06d60c 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -1902,7 +1902,7 @@ instrument_builtin_call (gimple_stmt_iterator *iter) return false; bool iter_advanced_p = false; - gcall *call = as_a <gcall *> (gsi_stmt (*iter)); + gcall *call = as_a <gcall *> (**iter); should be "fixed" by making instrument_builtin_call take a reference to the iterator so the above becomes