[llvm] r186119 - In response to dblaikie's comment on r186035, replacing the

[Adrian Prantl]

On Jul 12, 2013, at 1:54 PM, David Blaikie <dblaikie at gmail.com> wrote:

> On Thu, Jul 11, 2013 at 2:16 PM, Adrian Prantl <aprantl at apple.com> wrote:
>> Author: adrian
>> Date: Thu Jul 11 16:16:14 2013
>> New Revision: 186119
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=186119&view=rev
>> Log:
>> In response to dblaikie's comment on r186035, replacing the
>> (reduced LLVM IR) + (full source in comment)
>> with the
>> (full LLVM IR) + (reduced src in comment)
> 
> Thanks
> 
> Though I'm still curious if this is the simplest example - could you
> help me understand what the bug was & what this example is attempting
> to demonstrate?

This testcase only exists because Eric specifically requested one for this commit:
http://llvm.org/viewvc/llvm-project?rev=186014&view=rev

> Indirect parameters were an issue I fixed when I added the
> "isIndirect" flag, with the simple test case in
> test/DebugInfo/X86/arguments.ll
> 
> What is it that was still broken/that you fixed? (it might even make
> sense to just expand the arguments.ll test case to include an example
> that triggers your bug)

See below the discussion over in the main thread for this:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130708/180776.html
> 
> Also - tests that are simply "test we don't crash" are pretty low
> value - presumably rather than crashing there's some /correct/
> behavior we should have not just "do anything other than crash". We
> should be testing for whatever that correct/desired behavior is (the
> critical piece we were missing/that lead to the bug, anyway).

If you look at the commit history, this commit this testcase belongs to fixes a assertion/crash. That’s why the testcase tests that we don’t crash. If you are interested in a test case for the desired behavior, have a look at
http://llvm.org/viewvc/llvm-project?rev=185966&view=rev
there’s plenty.

-- adrian