On Mon, 17 Dec 2018 at 15:11, Jonathan Wakely wrote: > > On Mon, 17 Dec 2018 at 14:55, Nathan Sidwell <nat...@acm.org> wrote: > > > > On 12/14/18 10:55 AM, nick wrote: > > > Greetings All, > > > I was attempting to fix this bug: > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88395#add_comment > > > > > > and managed to track it down to expand_concept in constraint.cc. > > > > > > Seems that we are hitting this case in tsubst_expr for the NULL_TREE: > > > if (t == NULL_TREE || t == error_mark_node) > > > return t > > > and we only check result for error_mark_node. Is error_mark_node > > > equal to a NULL_TREE or is it something else? > > > > error_mark_node is a non-null pointer to a tree_node with ERROR_MARK as > > its tree code. (if you build with CXXFLAGS=-g3 you'll get macros > > available so gdb's 'p error_mark_node' will work) > > > > > If not than it seems > > > that we should change our check to: > > > if (result == error_mark_node || result == NULL_TREE) > > > > > > and it seems that this is also not checked it the other callers so it > > > should be fixed. > > > > Swapping the || arguments is not the fix -- it'll make no difference (in > > a well-formed program). > > I think Nick's just been very unclear about what he's suggesting. I > think the first snippet of code he shows is inside tsubst_expr and > shows that it can return either NULL or error_mark_node. > > The second snippet is his suggested fix for the caller of tsubst_expr > in expand_concept. That would have been a lot more helpful as a patch: > > --- a/gcc/cp/constraint.cc > +++ b/gcc/cp/constraint.cc > @@ -563,7 +563,7 @@ expand_concept (tree decl, tree args) > ++processing_template_decl; > tree result = tsubst_expr (def, args, tf_none, NULL_TREE, true); > --processing_template_decl; > - if (result == error_mark_node) > + if (result == error_mark_node || t == NULL_TREE) > return error_mark_node; > > /* And lastly, normalize it, check for implications, and save > > The point is that tsubst_expr can return NULL_TREE, we should check for it. > > Nick, please make more of an effort to explain yourself clearly, so > people can see what you're suggesting without having to read between > the lines. The standard way to show a proposed change is to show a > patch. If you'd sent the patch above and said "tsubst_expr can return > NULL_TREE" it would have taken us no time to understand.
Oh, and the answer to "Let me know if I'm missing something" is to test your suggested change and see if it breaks anything, and see if it fixes the bug.
