[clang] [Clang] Emit error for duplicate mangled names within a lambda (PR #107581)
Kerry McLaughlin via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 20 09:19:21 PDT 2024
kmclaughlin-arm wrote:
> For the lambda example, there are only three relevant calls to GetOrCreateLLVMFunction; one for each function with IsInDefinition false, but then only one with IsInDefinition true.
>
> It's not clear to me why the two cases are different, and I don't really want to add lambda-specific check without knowing why lambdas are special here. Maybe the right condition to check is something else.
>From what I've observed comparing the lambda and template test cases, the difference comes from how we create the list of Decls which can be deferred.
Both functions `foo()` from the template case in https://github.com/llvm/llvm-project/pull/107581#issuecomment-2343651051 and `function_params_normal_streaming()` from the lambda test case in this PR cannot be deferred by EmitGlobal here and must be emitted straight away (by calling GetOrCreateLLVMFunction):
```
if (MustBeEmitted(Global) && MayBeEmittedEagerly(Global)) {
EmitGlobalDefinition(GD);
addEmittedDeferredDecl(GD);
return;
```
For the template case this happens first, before calling EmitGlobal for `dispatcher()`. For lambdas the function arguments of the lambda are handled first. This changes whether the name is found later in EmitGlobal when adding the MangledName to the DeferredDeclsToEmit list:
```
StringRef MangledName = getMangledName(GD);
if (GetGlobalValue(MangledName) != nullptr) {
// The value has already been used and should therefore be emitted.
addDeferredDeclToEmit(GD);
...
} else {
// Otherwise, remember that we saw a deferred decl with this name. The
// first use of the mangled name will cause it to move into
// DeferredDeclsToEmit.
DeferredDecls[MangledName] = GD;
}
```
Although we'll set DeferredDecls with the mangled name from the lambda twice, it will only appear in DeferredDeclsToEmit once. In the other example, it will be added to the DeferredDeclsToEmit list with addDeferredDeclToEmit both times. When EmitDeferred is called later it uses this list and calls GetOrCreateLLVMFunction with IsForDefinition set to true.
I'm not sure adding a check specificially for lambdas is the right thing to do because of this. I considered trying to emit the error somewhere in lib/Sema, however I think this is too early to check whether functions will have the same mangled name.
Something I'm considering instead is to try and emit the error in EmitGlobal, where we set DeferredDecls for a given MangledName (around `DeferredDecls[MangledName] = GD` in the snippet above). I think this could be a better place to emit the error as we can look for whether GD has already been assigned to MangledName. I'm still investigating how to do this without causing failures in some of the existing tests which are using lambdas & templates.
I will also be away for the next two weeks but plan to pick this up again when I return.
https://github.com/llvm/llvm-project/pull/107581
More information about the cfe-commits
mailing list