max-winkler wrote: Let me briefly explain why I choosed this solution, because other approaches are indeed possible.
The current version of the tweak prints the base class method's signature to a string via ```c++ Decl::print (raw_ostream &Out) ``` However, qualifiers are not printed per default. There is an overload allowing to specify a `PrintingPolicy`, ```c++ Decl::print (raw_ostream &Out, const PrintingPolicy &Policy) ``` With `Policy` having `FullyQualify=true` and `Callbacks=PCB` with `PCB` like in `clang-tools-extra/clangd/AST.cpp:426`, this gives almost the desired line with all necessary qualifiers for parameters and return type, but also for the method name, like here: ```c++ ns::RetType ns::Base::method(ns::ParamType p) ``` It remains to remove `ns::Base::` in front of the method name. Several approaches are be possible: 1. Remove the qualifier of the method name by string manipulation, or, similar to the helper function `removePureVirtualSyntax`, by lex'ing that string, spot the tokens that belong to the qualifier and drop them. This is IMHO very error-prone. 2. Extend the DeclPrinter to allow different values in the FullyQualify-option for function name and input/output types. I guess changing a clang core component only for a clangd tweak is maybe not what people want :). 3. When calling `Decl::print` for a `CXXMethod` we land in `DeclPrinter::VisitFunctionDecl` [(here)](https://github.com/llvm/llvm-project/blob/e246c7f3836489a76c117f5fae09ae904fe14e50/clang/lib/AST/DeclPrinter.cpp#L681) where the actual printing logic can be found. I extracted the parts relevant for C++ method overrides and moved them to the `generateOverrideString` function in the tweak. This allows also to simply skip writing `virtual` and `= 0` which makes the helper `removePureVirtualSyntax` obsolete. Also, we can print parameter types and return type with the PrintingPolicy adding the correct qualifiers, and the method name separately without that policy. **Further remarks:** - There was also a tiny mistake in `removePureVirtualSyntax` which added a whitespace after the ampersand for parameters as reference giving `type & x` instead of `type &x` (clang-format). I fixed also the unit test which is complaining about this. - The method name inserted into the override string was obtained via `NamedDecl::getName()`. This triggers an assertion for methods that don't have an identifier. These are, for instance, all operators (`operator[]`, `operator==`, etc.). I changed this to obtain the name via `NamedDecl::getNameAsString()`. Also a test for overriding pure virtual operators is added. Please let me know if you want to see this in a separate PR. https://github.com/llvm/llvm-project/pull/237866 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
