[PATCH] D44598: [lit] Report line number for failed RUN command

Dan Liew via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 12 14:32:57 PDT 2018


delcypher added inline comments.


================
Comment at: utils/lit/lit/TestRunner.py:792
 
+    if cmd.commands[0].args[0] == ':':
+        if len(cmd.commands) != 1:
----------------
jdenny wrote:
> jdenny wrote:
> > delcypher wrote:
> > > jdenny wrote:
> > > > delcypher wrote:
> > > > > The null command is pretty esoteric but occasionally useful in normal shell script. What's the reason it's not supported?
> > > > > I guess we don't normally need it and it's a waste of time to implement it for the internal shell?
> > > > That would be my guess.  Do you object to implementing it for this purpose?
> > > I'm fine with you not implementing the null command. I was just curious. You could perhaps leave a `TODO` or `FIXME` commenting that we don't need to support it right now.
> > It seems we're miscommunicating.  This does implement the null command.  It just rejects the null command in a pipeline because I don't see a use for that right now.
> In case it helps to make this thread intelligible: I made my initial response here under the assumption that you were asking about the existing state of lit without my patch.  There, the null command indeed is not implemented.
> 
> The null command implementation is needed by this patch so that the internal shell reports run lines as well.
> 
> Any objection?
Ah sorry. I misread the code. I see now that your are adding support for the null command to the external shell in the case that there is no pipeline.  This isn't mentioned in the commit message so perhaps you should mention it?

I have no objection to the code here.


https://reviews.llvm.org/D44598





More information about the llvm-commits mailing list