[PATCH] D32218: X86AsmParser.cpp asserts: OperandStack.size() > 1 && "Too few operands."

[Andrew V. Tischenko via Phabricator via llvm-commits]

avt77 added inline comments.


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1341
+//      break;
 
     // If we're parsing an immediate expression, we don't expect a '['.
----------------
mcrosier wrote:
> avt77 wrote:
> > RKSimon wrote:
> > > Don't comment out code - delete it.
> > I'm waiting for the answer from @mcrosier: if we don't need this check at all then I'll remove the commented code and return the local variable initialization above to its origin place.
> > 
> > @mcrosier, could you comment this patch now?
> Can you think of an example where Tok starts with '.' and is *not* an identifier?
> 
> In other words, I'm thinking your change is causing this condition to be unconditionally false.  If that's the case, then the right solution might be to remove this check entirely.
> 
> Unfortunately, we don't have a lot of lit tests that cover the dot operator.  Have you tested this chance on something more substantial than the llvm test suite?
> 
No, I tested it as "check-all" only. Could you suggest something suitable?
And of course I'm ready to keep the check like it's written inside the commented code. I don't know the example of such token but it was written for some reason that's why I suggest to use the commented version. What do you think about?


https://reviews.llvm.org/D32218