llvm dev - Jan 2015 - [LLVMdev] Should I commit "IR: Move MDLocation into place" before or after 3.6 branch?

> On 2015-Jan-14, at 11:26, Adrian Prantl <aprantl at apple.com> wrote:
> 
> 
>> On Jan 14, 2015, at 11:12 AM, David Blaikie <dblaikie at
gmail.com> wrote:
>> 
>> 
>> 
>> On Wed, Jan 14, 2015 at 11:04 AM, Adrian Prantl <aprantl at
apple.com> wrote:
>> One small request: Omitting the column field from MDLocation if it is 0
is fine, because it won’t make it into the line table. Omitting the line if it
is 0 is not, because it gives the wrong impression that the line is being
ignored, which is not the case. Line 0 will be emitted in the line table and has
special semantics (Line 0 in DWARF may be used to mark compiler-generated code
that has no corresponding source line number).
>> 
>> I don't know - I think I might be OK with it being implicitly zero
(I think that's the column behavior - if the column is never set it's
just zero in the line table? (which is "no specific column") - that
seems consistent with zero line ("no actual line in the source code”))
> 
> The difference is that column 0 won’t be emitted into the line table, but
line 0 will be — which is why I think it would less misleading to mirror this
behavior in the IR.
What about this:

  - Still have `MDLocation(scope: !0)` imply `line: 0`.
  - Change printout to include `line:` even when it's `0`.

There's a sane default, so we shouldn't have to specify it.
However, the value is important and relevant, so the assembly
writer *does* print it out.

Thoughts?

(Frankly I still prefer `line: 0` being omitted, but it did
seem kind of funny when I was updating tests that CHECK for
`line: 0` that they now check for `MDLocation(scope:`, so I
guess on balance I'm indifferent here.)

On Wed, Jan 14, 2015 at 11:39 AM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:
>
> > On 2015-Jan-14, at 11:26, Adrian Prantl <aprantl at apple.com>
wrote:
> >
> >
> >> On Jan 14, 2015, at 11:12 AM, David Blaikie <dblaikie at
gmail.com> wrote:
> >>
> >>
> >>
> >> On Wed, Jan 14, 2015 at 11:04 AM, Adrian Prantl <aprantl at
apple.com>
> wrote:
> >> One small request: Omitting the column field from MDLocation if it
is 0
> is fine, because it won’t make it into the line table. Omitting the line if
> it is 0 is not, because it gives the wrong impression that the line is
> being ignored, which is not the case. Line 0 will be emitted in the line
> table and has special semantics (Line 0 in DWARF may be used to mark
> compiler-generated code that has no corresponding source line number).
> >>
> >> I don't know - I think I might be OK with it being implicitly
zero (I
> think that's the column behavior - if the column is never set it's
just
> zero in the line table? (which is "no specific column") - that
seems
> consistent with zero line ("no actual line in the source code”))
> >
> > The difference is that column 0 won’t be emitted into the line table,
> but line 0 will be — which is why I think it would less misleading to
> mirror this behavior in the IR.
>
> What about this:
>
>   - Still have `MDLocation(scope: !0)` imply `line: 0`.
>   - Change printout to include `line:` even when it's `0`.
>
> There's a sane default, so we shouldn't have to specify it.
> However, the value is important and relevant, so the assembly
> writer *does* print it out.
>
> Thoughts?
>
> (Frankly I still prefer `line: 0` being omitted, but it did
> seem kind of funny when I was updating tests that CHECK for
> `line: 0` that they now check for `MDLocation(scope:`, so I
> guess on balance I'm indifferent here.)

I'm pretty much in agreement with you on all counts there.

Omitting it is important for easy of writability of test cases.

I'm not sure it'll substantially hinder readability to have it omitted,
so
I'm not in any hurry to have it print by default - but perhaps others feel
more strongly.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150114/e910bd32/attachment.html>

> On Jan 14, 2015, at 11:39 AM, Duncan P. N. Exon Smith <dexonsmith at
apple.com> wrote:
> 
> 
>> On 2015-Jan-14, at 11:26, Adrian Prantl <aprantl at apple.com>
wrote:
>> 
>> 
>>> On Jan 14, 2015, at 11:12 AM, David Blaikie <dblaikie at
gmail.com> wrote:
>>> 
>>> 
>>> 
>>> On Wed, Jan 14, 2015 at 11:04 AM, Adrian Prantl <aprantl at
apple.com> wrote:
>>> One small request: Omitting the column field from MDLocation if it
is 0 is fine, because it won’t make it into the line table. Omitting the line if
it is 0 is not, because it gives the wrong impression that the line is being
ignored, which is not the case. Line 0 will be emitted in the line table and has
special semantics (Line 0 in DWARF may be used to mark compiler-generated code
that has no corresponding source line number).
>>> 
>>> I don't know - I think I might be OK with it being implicitly
zero (I think that's the column behavior - if the column is never set
it's just zero in the line table? (which is "no specific column")
- that seems consistent with zero line ("no actual line in the source
code”))
>> 
>> The difference is that column 0 won’t be emitted into the line table,
but line 0 will be — which is why I think it would less misleading to mirror
this behavior in the IR.
> 
> What about this:
> 
> - Still have `MDLocation(scope: !0)` imply `line: 0`.
> - Change printout to include `line:` even when it's `0`.
> 
> There's a sane default, so we shouldn't have to specify it.
> However, the value is important and relevant, so the assembly
> writer *does* print it out.
works for me!

-- adrian
> 
> Thoughts?
> 
> (Frankly I still prefer `line: 0` being omitted, but it did
> seem kind of funny when I was updating tests that CHECK for
> `line: 0` that they now check for `MDLocation(scope:`, so I
> guess on balance I'm indifferent here.)

> On 2015-Jan-14, at 11:50, Adrian Prantl <aprantl at apple.com> wrote:
> 
>> 
>> On Jan 14, 2015, at 11:39 AM, Duncan P. N. Exon Smith <dexonsmith at
apple.com> wrote:
>> 
>> 
>>> On 2015-Jan-14, at 11:26, Adrian Prantl <aprantl at
apple.com> wrote:
>>> 
>>> 
>>>> On Jan 14, 2015, at 11:12 AM, David Blaikie <dblaikie at
gmail.com> wrote:
>>>> 
>>>> 
>>>> 
>>>> On Wed, Jan 14, 2015 at 11:04 AM, Adrian Prantl <aprantl at
apple.com> wrote:
>>>> One small request: Omitting the column field from MDLocation if
it is 0 is fine, because it won’t make it into the line table. Omitting the line
if it is 0 is not, because it gives the wrong impression that the line is being
ignored, which is not the case. Line 0 will be emitted in the line table and has
special semantics (Line 0 in DWARF may be used to mark compiler-generated code
that has no corresponding source line number).
>>>> 
>>>> I don't know - I think I might be OK with it being
implicitly zero (I think that's the column behavior - if the column is never
set it's just zero in the line table? (which is "no specific
column") - that seems consistent with zero line ("no actual line in
the source code”))
>>> 
>>> The difference is that column 0 won’t be emitted into the line
table, but line 0 will be — which is why I think it would less misleading to
mirror this behavior in the IR.
>> 
>> What about this:
>> 
>> - Still have `MDLocation(scope: !0)` imply `line: 0`.
>> - Change printout to include `line:` even when it's `0`.
>> 
>> There's a sane default, so we shouldn't have to specify it.
>> However, the value is important and relevant, so the assembly
>> writer *does* print it out.
> 
> works for me!
r226046
> -- adrian
>> 
>> Thoughts?
>> 
>> (Frankly I still prefer `line: 0` being omitted, but it did
>> seem kind of funny when I was updating tests that CHECK for
>> `line: 0` that they now check for `MDLocation(scope:`, so I
>> guess on balance I'm indifferent here.)