<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 23 May 2017 at 15:11, Tim Northover via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">> +      if (!isImaginary) {<br>
><br>
> Shouldn't this be "if (s != ThisTokEnd)"? (That is, if this was not a valid builtin suffix, reset the suffix information.)<br>
<br>
</span>I think they're equivalent here:<br>
<br>
    !isImaginary => UDL is _ or time thing => extra crap on the end.<br>
    extra crap on the end => it's not a legitimate C++14 imaginary => invalid as no _.<br>
<br>
so I went for the same test as when the data is used.<br></blockquote><div><br></div><div>I think they're equivalent by coincidence, but the right test is that we consumed the entire suffix. (If, say, C++20 adds an "in" suffix for "inches", we shouldn't start treating that as an imaginary number if no 'operator""in' is declared.)</div><div><br></div><div>Maybe it would be clearer to store a separate "valid as non-UDL suffix" flag instead of resetting these flags to false if the suffix is not valid as a non-UDL suffix?</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
But I'm not committed to that, so I'm happy to switch if you're unconvinced. I don't think I'll be able to test the change though, even if there is a suffix where it matters (I can't think of anywhere those fields matter in the UDL but not imaginary case).<br></blockquote><div><br></div><div>Right, I don't think this makes a difference today, because the only ud-suffixes we let through here are "i", "if", and "il", all of which are also valid builtin suffixes. But as noted above, this will do the wrong thing if we permit more ud-suffixes beginning with "i" or "j".</div></div></div></div>