linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Cc: Sparse Mailing-list <linux-sparse@vger.kernel.org>
Subject: Re: [PATCH 15/17] scope: give a scope for labels & gotos
Date: Mon, 13 Apr 2020 12:32:41 -0700	[thread overview]
Message-ID: <CAHk-=wjMSeVVQHZ23=HJ3V-yYPooeuHNHmZtexUVRKg_e5VMXA@mail.gmail.com> (raw)
In-Reply-To: <20200413185452.pgj75pj5g7a42kik@ltop.local>

On Mon, Apr 13, 2020 at 11:54 AM Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> Yes, I agree and in fact (if I understand you correctly) it was what
> I tried first, mainly because it was "conceptualy neat" and simpler.
> But then it wasn't working correctly in all situations and I
> convinced myself it couldn't. The problem was with code like:
>         void foo(void)
>         {
>                 ... = ({ ...  goto out; ... });
>
>         out:
>                 ...;
>         }
>
> In this case, when 'goto out' is parsed, the corresponding label
> symbol would be created in the inner scope and later when the label
> is defined the symbol lookup will only look in the outer scope, see
> nothing and declare another symbol for it

Oh, yeah, I see.

And that's just because of how we basically do the same thing at
"goto" time as we do at "label definition" time.

Both basically create the symbol label.

Which was simple, and worked, and meant that you never had to look
anything up, because they automatically just did the right thing - and
the use and scope is symmetric.

And the reason it does that, is that labels - unlike every other
symbol - aren't declared before use. So it's a hacky solution, and it
works.

And by "it works", I mean "doesn't really work all that well", because
clearly all the _other_ patches in your series were about the fact
that it also meant that we were horrible at the whole "label was never
defined in the first place" case.

But with the scoping change, the use and scope isn't symmetric any
more, and the "create symbol both at use and at definition" doesn't
work.

I _feel_ like the fix to that should be that the only thing that
creates the actual symbol is the label definition, and that the goto
should only ever use the 'ident' and we'd tie the two together later.

But yeah, that "tie the two together later" may not work, simply
because scoping is so tightly tied to parsing in sparse.

So maybe your approach is the best one.

It feels hacky and wrong, but maybe that just fundamentally comes from
labels having that very special "use = implicit declaration" thing.

              Linus

  reply	other threads:[~2020-04-13 19:33 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-13 16:15 [PATCH 00/17] detect invalid branches at evaluation time Luc Van Oostenryck
2020-04-13 16:15 ` [PATCH 01/17] bad-goto: add testcase for 'jump inside discarded expression statement' Luc Van Oostenryck
2020-04-13 16:15 ` [PATCH 02/17] bad-goto: add testcases for linearization of invalid labels Luc Van Oostenryck
2020-04-13 16:15 ` [PATCH 03/17] bad-goto: add more testcases Luc Van Oostenryck
2020-04-13 16:15 ` [PATCH 04/17] bad-goto: do not linearize if the IR will be invalid Luc Van Oostenryck
2020-04-13 16:15 ` [PATCH 05/17] bad-goto: reorg test in evaluate_goto_statement() Luc Van Oostenryck
2020-04-13 16:15 ` [PATCH 06/17] bad-goto: simplify testing of undeclared labels Luc Van Oostenryck
2020-04-13 16:15 ` [PATCH 07/17] bad-goto: do not linearize function with " Luc Van Oostenryck
2020-04-13 16:15 ` [PATCH 08/17] bad-goto: catch labels with reserved names Luc Van Oostenryck
2020-04-13 16:15 ` [PATCH 09/17] scope: no memset() needed after __alloc_scope() Luc Van Oostenryck
2020-04-13 16:15 ` [PATCH 10/17] scope: move scope opening/ending inside compound_statement() Luc Van Oostenryck
2020-04-13 16:15 ` [PATCH 11/17] scope: make function scope the same as the body block scope Luc Van Oostenryck
2020-04-13 16:16 ` [PATCH 12/17] scope: s/{start,end}_symbol_scope/{start,end}_block_scope/ Luc Van Oostenryck
2020-04-13 16:16 ` [PATCH 13/17] scope: let labels have their own scope Luc Van Oostenryck
2020-04-13 17:30   ` Linus Torvalds
2020-04-13 16:16 ` [PATCH 14/17] scope: add is_in_scope() Luc Van Oostenryck
2020-04-13 16:16 ` [PATCH 15/17] scope: give a scope for labels & gotos Luc Van Oostenryck
2020-04-13 17:52   ` Linus Torvalds
2020-04-13 18:54     ` Luc Van Oostenryck
2020-04-13 19:32       ` Linus Torvalds [this message]
2020-04-13 20:00         ` Luc Van Oostenryck
2020-04-13 22:40         ` Linus Torvalds
2020-04-13 23:39           ` Luc Van Oostenryck
2020-04-14  7:49             ` Luc Van Oostenryck
2020-04-14 18:19               ` Linus Torvalds
2020-04-14 23:09                 ` Luc Van Oostenryck
2020-04-15  0:59                   ` Linus Torvalds
2020-05-14 22:22                     ` Luc Van Oostenryck
2020-04-13 16:16 ` [PATCH 16/17] bad-goto: catch gotos inside expression statements Luc Van Oostenryck
2020-04-13 16:16 ` [PATCH 17/17] bad-goto: cleanup evaluate_goto() Luc Van Oostenryck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHk-=wjMSeVVQHZ23=HJ3V-yYPooeuHNHmZtexUVRKg_e5VMXA@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=linux-sparse@vger.kernel.org \
    --cc=luc.vanoostenryck@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).