[PATCH] D104777: PR50767: clear non-distinct debuginfo for function with nodebug definition after undecorated declaration

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D104777#2836327 <https://reviews.llvm.org/D104777#2836327>, @aaron.ballman wrote:

> In D104777#2835965 <https://reviews.llvm.org/D104777#2835965>, @dblaikie wrote:
>
>> @aaron.ballman Do we have attributes/infrastructure for attributes that have to be the same from their first declaration or at least from their first call? I'm wondering if it might be simpler/better to require nodebug to be provided earlier, rather than fixing it up after the fact like this.
>>
>> (for instance, requiring it to be attributed on the declaration would ensure we don't create call_site descriptions for calls to nodebug functions - which would save some debug info size)
>
> We don't have any attributes that have to be the same "from their first call" that I'm aware of. But we do have attributes that need to be the same on all declarations. IIRC, the `section` and `code_seg` attributes both have to be specified on the initial declaration in some circumstances, and `[[noreturn]]` needs to be on the first declaration we see. We don't have any tablegen machinery to enforce this, the logic needs to be manually written. I think it'd live somewhere around `Sema::mergeDeclAttributes()` or `Sema::MergeFunctionDecl()` most likely.

Yeah, all that sounds reasonable to me - @brunodefraine could you look into supporting nodebug in a similar way as @aaron.ballman has described here?

> Fixing up after the fact would be a bit strange though. A declaration is typically considered marked by an attribute from the point of declaration including the attribute onward, not backwards. e.g., https://godbolt.org/z/4E1Ya764n I won't say we *never* do this back propagation, but I would say it's a sign that the attribute is venturing into novel, perhaps dangerous territory. For example with this patch, an AST matcher won't see the `nodebug` attribute on the initial declaration of `t1()` and so it won't react to it the same way that codegen is being changed.

Yeah, fair concerns for sure - thanks for the tips!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104777/new/

https://reviews.llvm.org/D104777