dblaikie added inline comments.
================
Comment at: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c:34-39
+static int go(a) int a;
+{
+ return glob + a;
+}
+
+
----------------
hoy wrote:
> dblaikie wrote:
> > hoy wrote:
> > > dblaikie wrote:
> > > > Does this need to be down here? Or would the code be a well exercised
> > > > if it was up next to the go declaration above?
> > > Yes, it needs to be here. Otherwise it will just like the function `bar`
> > > above that doesn't get a uniquefied name. I think moving the definition
> > > up to right after the declaration hides the declaration.
> > Not sure I follow - do you mean that if the go declaration and go
> > definition were next to each other, this test would (mechanically speaking)
> > not validate what the patch? Or that it would be less legible, but still
> > mechanically correct?
> >
> > I think it would be (assuming it's still mechanically correct) more legible
> > to put the declaration next to the definition - the comment describes why
> > the declaration is significant/why the definition is weird, and seeing all
> > that together would be clearer to me than spreading it out/having to look
> > further away to see what's going on.
> When the `go` declaration and `go` definition were next to each other, the go
> function won't get a uniqufied name at all. The declaration will be
> overwritten by the definition. Only when the declaration is seen by others,
> such the callsite in `baz`, the declaration makes a difference by having the
> callsite use a uniqufied name.
>
>
Ah! Interesting, good to know.
Is that worth supporting, I wonder? I guess it falls out for free/without
significant additional complexity. I worry about the subtlety of the additional
declaration changing the behavior here... might be a bit surprising/subtle. But
maybe no nice way to avoid it either.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98799/new/
https://reviews.llvm.org/D98799
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
