| Message ID | 4F3CC4FA.2070209@st.com |
|---|---|
| State | New |
| Headers | show |
> > > On 02/15/2012 06:03 PM, Joseph S. Myers wrote: >> On Wed, 15 Feb 2012, Christian Bruel wrote: >> >>> Removal of this NOP_EXPR if !CAN_HAVE_LOCATION_P fixes the problem. >>> looks safe from my testing, because the loc is inserted using >>> 'protected_set_expr_location', whereas no loc for a constant case >>> doesn't seem to introduce new problems with the debugging information. >>> But in case I also added a few paranoid gcc_assert. >> >> Is it safe to set TREE_NO_WARNING without that check? I'd have thought >> the check was to avoid setting TREE_NO_WARNING on a shared node when >> warnings are to be disabled in only one context where that node is used. In fact, we can't omit the TREE_NO_WARNING is !CAN_HAVE_LOCATION_P, the tentative bellow causes a regression on middle-end/13325. What I'm unsure is why we couldn't have a TREE_NO_WARNING on a !CAN_HAVE_LOCATION_P. This seems necessary on some cases without using a NOP_EXPR. >> > > OK, I see, we can still keep the check, like with : > > Index: c-common.c > =================================================================== > --- c-common.c (revision 184301) > +++ c-common.c (working copy) > @@ -1435,12 +1435,9 @@ > have been done by this point, so remove them again. */ > nowarning |= TREE_NO_WARNING (ret); > STRIP_TYPE_NOPS (ret); > - if (nowarning && !TREE_NO_WARNING (ret)) > - { > - if (!CAN_HAVE_LOCATION_P (ret)) > - ret = build1 (NOP_EXPR, TREE_TYPE (ret), ret); > - TREE_NO_WARNING (ret) = 1; > - } > + if (nowarning && CAN_HAVE_LOCATION_P (ret)) > + TREE_NO_WARNING (ret) = 1; > + > if (ret != expr) > protected_set_expr_location (ret, loc); > return ret; > > We propagate the flag to the new node for non-constant folded > expressions, or for a converted constant (so NOT_EXPR is not stripped), > > Does it make sense to set TREE_NO_WARNING for a not-converted folded > constant ? it seems that in the proposal, a warning before the fold is > OK, and a warning after the fold on the new expression is still to be > honored. The NO_WARNING flag on an non-converted constant that is folded > looks unnecessary. I tried to forge different scenarios, were the result > of the folded constant would force a duplicate warning, but no success > to find a regression case. > > Thanks > > Christian
On Thu, Feb 16, 2012 at 12:49 PM, Christian Bruel <christian.bruel@st.com> wrote: > >> >> >> On 02/15/2012 06:03 PM, Joseph S. Myers wrote: >>> On Wed, 15 Feb 2012, Christian Bruel wrote: >>> >>>> Removal of this NOP_EXPR if !CAN_HAVE_LOCATION_P fixes the problem. >>>> looks safe from my testing, because the loc is inserted using >>>> 'protected_set_expr_location', whereas no loc for a constant case >>>> doesn't seem to introduce new problems with the debugging information. >>>> But in case I also added a few paranoid gcc_assert. >>> >>> Is it safe to set TREE_NO_WARNING without that check? I'd have thought >>> the check was to avoid setting TREE_NO_WARNING on a shared node when >>> warnings are to be disabled in only one context where that node is used. > > In fact, we can't omit the TREE_NO_WARNING is !CAN_HAVE_LOCATION_P, the > tentative bellow causes a regression on middle-end/13325. > > What I'm unsure is why we couldn't have a TREE_NO_WARNING on a > !CAN_HAVE_LOCATION_P. This seems necessary on some cases without using a > NOP_EXPR. You cannot have it on possibly shared tree nodes. CAN_HAVE_LOCATION_P tree nodes are not shared (the reverse is not true). Richard.
First, if there isn't a bug in Bugzilla for this problem please file one so it's properly tracked if it takes a while to work out how to solve it. As I understand it from your testcases, it's a matter of certain code that is not valid ISO C but you would like to be accepted unless -pedantic, by analogy with other such code that is accepted. On Thu, 16 Feb 2012, Christian Bruel wrote: > What I'm unsure is why we couldn't have a TREE_NO_WARNING on a > !CAN_HAVE_LOCATION_P. This seems necessary on some cases without using a > NOP_EXPR. Richard explained this. What is the code adding TREE_NO_WARNING in the first place (before folding) for your testcase? It may sometimes be safe to copy constants with copy_node to put TREE_NO_WARNING on the copies. But you'd still need to be careful because there are some tree-node-equality comparisons for constants, against truthvalue_true_node and truthvalue_false_node at least, so those might need to be relaxed a bit to allow any non-overflowed integer-type integer constant with the right value.
On 02/16/2012 02:14 PM, Joseph S. Myers wrote: > First, if there isn't a bug in Bugzilla for this problem please file one > so it's properly tracked if it takes a while to work out how to solve it. OK, tracked with PR52283 > As I understand it from your testcases, it's a matter of certain code that > is not valid ISO C but you would like to be accepted unless -pedantic, by > analogy with other such code that is accepted. > > On Thu, 16 Feb 2012, Christian Bruel wrote: > >> What I'm unsure is why we couldn't have a TREE_NO_WARNING on a >> !CAN_HAVE_LOCATION_P. This seems necessary on some cases without using a >> NOP_EXPR. > > Richard explained this. thanks for the explanation, Christian
Index: c-common.c =================================================================== --- c-common.c (revision 184301) +++ c-common.c (working copy) @@ -1435,12 +1435,9 @@ have been done by this point, so remove them again. */ nowarning |= TREE_NO_WARNING (ret); STRIP_TYPE_NOPS (ret); - if (nowarning && !TREE_NO_WARNING (ret)) - { - if (!CAN_HAVE_LOCATION_P (ret)) - ret = build1 (NOP_EXPR, TREE_TYPE (ret), ret); - TREE_NO_WARNING (ret) = 1; - } + if (nowarning && CAN_HAVE_LOCATION_P (ret)) + TREE_NO_WARNING (ret) = 1; + if (ret != expr) protected_set_expr_location (ret, loc); return ret;