[cherry-pick][stable/20240723] [lldb][ClangExpressionParser] Implement ExternalSemaSource::ReadUndefinedButUsed #9161
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ernalASTSources (llvm#104799) When we use `SemaSourceWithPriorities` as the `ASTContext`s ExternalASTSource, we allocate a `ClangASTSourceProxy` (via `CreateProxy`) and two `ExternalASTSourceWrapper`. Then we push these sources into a vector in `SemaSourceWithPriorities`. The allocated `SemaSourceWithPriorities` itself will get properly deallocated because the `ASTContext` wraps it in an `IntrusiveRefCntPtr`. But the three sources we allocated earlier will never get released. This patch fixes this by mimicking what `MultiplexExternalSemaSource` does (which is what `SemaSourceWithPriorities` is based on anyway). I.e., when `SemaSourceWithPriorities` gets constructed, it increments the use count of its sources. And on destruction it decrements them. Similarly, to make sure we dealloacted the `ClangASTProxy` properly, the `ExternalASTSourceWrapper` now assumes shared ownership of the underlying source. (cherry picked from commit 770cd24)
…inedButUsed (llvm#104817) While parsing an expression, Clang tries to diagnose usage of decls (with possibly non-external linkage) for which it hasn't been provided with a definition. This is the case, e.g., for functions with parameters that live in an anonymous namespace (those will have `UniqueExternal` linkage, this is computed [here in computeTypeLinkageInfo](https://github.com/llvm/llvm-project/blob/ea8bb4d633683f5cbfd82491620be3056f347a02/clang/lib/AST/Type.cpp#L4647-L4653)). Before diagnosing such situations, Clang calls `ExternalSemaSource::ReadUndefinedButUsed`. The intended use of this API is to extend the set of "used but not defined" decls with additional ones that the external source knows about. However, in LLDB's case, we never provide `FunctionDecl`s with a definition, and instead rely on the expression parser to resolve those symbols by linkage name. Thus, to avoid the Clang parser from erroring out in these situations, this patch implements `ReadUndefinedButUsed` which just removes the "undefined" non-external `FunctionDecl`s that Clang found. We also had to add an `ExternalSemaSource` to the `clang::Sema` instance LLDB creates. We previously didn't have any source on `Sema`. Because we add the `ExternalASTSourceWrapper` here, that means we'd also technically be adding the `ClangExpressionDeclMap` as an `ExternalASTSource` to `Sema`, which is fine because `Sema` will only be calling into the `ExternalSemaSource` APIs (though nothing currently strictly enforces this, which is a bit worrying). Note, the decision for whether to put a function into `UndefinedButUsed` is done in [Sema::MarkFunctionReferenced](https://github.com/llvm/llvm-project/blob/ea8bb4d633683f5cbfd82491620be3056f347a02/clang/lib/Sema/SemaExpr.cpp#L18083-L18087). The `UniqueExternal` linkage computation is done in [getLVForNamespaceScopeDecl](https://github.com/llvm/llvm-project/blob/ea8bb4d633683f5cbfd82491620be3056f347a02/clang/lib/AST/Decl.cpp#L821-L833). Fixes llvm#104712 (cherry picked from commit 8056d92)
This recently added test is failing on Windows with: ``` c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\lldb.exe --no-lldbinit -S C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/build/tools/lldb\test\Shell\lit-lldb-init-quiet C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\Expr\Output\TestAnonNamespaceParamFunc.cpp.tmp -o run -o "expression func(a)" -o exit | c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\filecheck.exe C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\Shell\Expr\TestAnonNamespaceParamFunc.cpp executed command: 'c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\lldb.exe' --no-lldbinit -S 'C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/build/tools/lldb\test\Shell\lit-lldb-init-quiet' 'C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\tools\lldb\test\Shell\Expr\Output\TestAnonNamespaceParamFunc.cpp.tmp' -o run -o 'expression func(a)' -o exit .---command stderr------------ | TestAnonNamespaceParamFunc.cpp.tmp :: Class 'tagARRAYDESC' has a member 'tdescElem' of type 'tagTYPEDESC' which does not have a complete definition.error: TestAnonNamespaceParamFunc.cpp.tmp :: Class 'tagARRAYDESC' has a member 'tdescElem' of type 'tagTYPEDESC' which does not have a complete definition. | (lldb) TestAnonNamespaceParamFunc.cpp.tmp :: Class 'std::partial_ordering' has a member 'less' of type 'std::partial_ordering' which does not have a complete definition.error: TestAnonNamespaceParamFunc.cpp.tmp :: Class 'std::partial_ordering' has a member 'less' of type 'std::partial_ordering' which does not have a complete definition. | (lldb) TestAnonNamespaceParamFunc.cpp.tmp :: Class 'std::strong_ordering' has a member 'less' of type 'std::strong_ordering' which does not have a complete definition.error: TestAnonNamespaceParamFunc.cpp.tmp :: Class 'std::strong_ordering' has a member 'less' of type 'std::strong_ordering' which does not have a complete definition. | (lldb) TestAnonNamespaceParamFunc.cpp.tmp :: Class 'std::weak_ordering' has a member 'less' of type 'std::weak_ordering' which does not have a complete definition.error: TestAnonNamespaceParamFunc.cpp.tmp :: Class 'std::weak_ordering' has a member 'less' of type 'std::weak_ordering' which does not have a complete definition. | (lldb) error: Couldn't look up symbols: | int func(struct `anonymous namespace'::InAnon) | Hint: The expression tried to call a function that is not present in the target, perhaps because it was optimized out by the compiler. `----------------------------- executed command: 'c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\filecheck.exe' 'C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\Shell\Expr\TestAnonNamespaceParamFunc.cpp' .---command stderr------------ | C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\Shell\Expr\TestAnonNamespaceParamFunc.cpp:10:11: error: CHECK: expected string not found in input | // CHECK: (int) $0 = 15 | ^ | <stdin>:16:26: note: scanning from here | (lldb) expression func(a) | ^ ``` So the function is still not callable. But AFAICT, this is not a regression, since this function wasn't callable prior to the patch anyway. I currently do not have a Windows setup to test this on, so XFAIL for now. (cherry picked from commit 8d712b4)
|
@swift-ci test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cherry-picks for cleaning up the
ExternalASTSourceallocations and fixing an expression parser bug.Addresses rdar://69379373