From 19813544766f74a10f403c9e474306c0f7b61a18 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Fri, 16 Oct 2026 07:20:21 +0000 Subject: [PATCH] fix(cpp): of two overloads alike but for a `const`, like protobuf's `DynamicCastMessage(const MessageLite*)` and `DynamicCastMessage(MessageLite*)`, a call reaches the one C++ picks (#2618) Overload fit by argument type (cppArgumentTypeChoice) scored a `const T*` parameter and its `T*` twin alike, and the one declared first won: protobuf's `DynamicCastMessage(ptr.get())` on a `std::shared_ptr` took the `const MessageLite*` overload. - cppConstTwinChoice: of the best-fitting overload's twins, alike but for the `const` of a pointer or reference parameter, the one that adds `const` to the fewest arguments. - cppWrittenTypeFit: a pointer or reference to a non-const type never takes a const object, nor a reference a value a call returns (cppNamesObject tells a named object from a call's result); an object is no pointer, nor a pointer to characters a pointer to another type (or the reverse). None of that for an argument of a template parameter's type, and the pointer and `const` rules not for a parameter the function's own template parameter types (it deduces the `const`). - The argument's type keeps a `const` its declaration writes in front (`const Buffer* b`, `constexpr char key[]`), which the declarator regex leaves out; an array is a pointer to its element; `&ref` is a pointer to the referee; cppLibraryElementOf keeps a `const` in a holder's template argument (`std::shared_ptr`). - Template headers are read 8 lines up, not 3 (fmt's `join`). A/B on the cgstd kit against main 0893606e: leveldb and redis identical; fmt 14, rocksdb 25 and protobuf 194 calls retargeted, each triaged against C++'s choice (const twins, objects and arrays off the wrong pointer overloads, temporaries to `T&&`). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- __tests__/cpp-overload-constness.test.ts | 195 +++++++++++++++++++++++ src/resolution/cpp-type-aliases.ts | 3 +- src/resolution/name-matcher.ts | 192 +++++++++++++++++++--- 4 files changed, 367 insertions(+), 25 deletions(-) create mode 100644 __tests__/cpp-overload-constness.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index fe02cdb9..1a6d968a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,7 +101,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In C++, a call on a variable, parameter or member declared as one of your classes, like `iter->Valid()` on an `Iterator* iter` or `it.Valid()` on a `const Iterator& it`, now links to that method of the class C++ itself would find for the type where the call is written, or of a class it inherits from. Before, the method was picked by the type's last name, so a class of the same name nested elsewhere could win: in facebook/rocksdb, calls on an `Iterator` reached `MemTableRep::Iterator`, which declares `Valid` and `Next` itself while the `Iterator` the code means inherits them, google/leveldb's reached `SkipList::Iterator`, and in protocolbuffers/protobuf the Java generator's `context->options()` reached the hpb generator's `Context` and `text_parser.Parse(…)` on a `TextFormat::Parser` reached the `.proto` parser. When that class and the classes it derives from have no method of that name, the call no longer links to an unrelated class's method that shares it, as protobuf's `field->full_name()` on a `FieldDescriptor*` did. A type is also found through a `using` declaration the file writes, like protobuf's `using google::protobuf::Descriptor;`, before a class of the same name declared at global scope elsewhere in the repository; this also applies to base classes and to the type a smart pointer holds. A type written from the global scope, like `::upb::Arena& arena`, is looked up there, so a namespace's own `upb::Arena` is not taken for it, nor is an alias of that name the calling function declares. A type written as a member of a class template, like `SkipList::Iterator iter` in leveldb's and RocksDB's skip-list tests, is now read whole, so its calls reach the skip list's own iterator rather than the general `Iterator`, and RocksDB's skip-list memtable iterator no longer links its calls on the skip list's iterator to its own methods; a standard container's `iterator` written this way is the library's, never a class of yours named `iterator`. Re-index C++ projects after upgrading. - In C++, a method defined by a macro of the same file is now indexed where the file invokes the macro, so calls to it link. protocolbuffers/protobuf's descriptor classes only declare most of their accessors and define them further down with macros like `PROTOBUF_DEFINE_ACCESSOR(FieldDescriptor, number, int)`, and its reflection defines `GetInt32`, `SetInt32` and the like by pasting names together in a macro. Those methods had no node at all, so calls like `field->number()`, `descriptor->full_name()` or `reflection->GetInt32(…)` linked to nothing. Calls through what such a method returns, like `field->containing_type()->full_name()`, now link too. A macro defined in another file, and a method declared but defined nowhere in the repository, are still not indexed. Re-index C++ projects after upgrading. - In C++, an explicit specialization of a function or member function template, like protocolbuffers/protobuf's `template <> bool WireFormatLite::ReadPrimitive(…)` or facebook/rocksdb's `template <> int64_t Deserialize(…)`, is now indexed under its template's name, so search finds it and calls link to it. Before, it kept its template arguments in its name, and one with a `::` among them was named after the last part, like `TYPE_INT32>`. A call that writes template arguments, like `Deserialize(src, offset)`, links to the specialization written with the same ones, and otherwise to the template itself, as fmt's `num_bits()` does; a call that writes none links to a specialization only when nothing else has that name. Re-index C++ projects after upgrading. -- In C++, a call to an overloaded function outside a class now links to the overload its arguments fit, by how many it passes and the types they have where the call writes them, as a call to a method already did: protocolbuffers/protobuf's `ParseWithLimit(limit, data, result)` on an `int32_t result` reaches the overload taking an `int32_t&`, and facebook/rocksdb's `NewLRUCache(cache_opts)` the one taking an `LRUCacheOptions`. Before, the overload nearest the call was taken. Such a call is never linked to an overload only another source file can see, like another `.cc` file's `static` function: it now links to the one its own file sees, where before it often linked to nothing. A call to a name that both a function and a class have reaches the function, which hides the class, and a parameter named like the function it belongs to, like protobuf's `std::string u64(uint64_t u64)`, now has its own type. Re-index C++ projects after upgrading. +- In C++, a call to an overloaded function outside a class now links to the overload its arguments fit, by how many it passes and the types they have where the call writes them, as a call to a method already did: protocolbuffers/protobuf's `ParseWithLimit(limit, data, result)` on an `int32_t result` reaches the overload taking an `int32_t&`, and facebook/rocksdb's `NewLRUCache(cache_opts)` the one taking an `LRUCacheOptions`. Before, the overload nearest the call was taken. Such a call is never linked to an overload only another source file can see, like another `.cc` file's `static` function: it now links to the one its own file sees, where before it often linked to nothing. A call to a name that both a function and a class have reaches the function, which hides the class, and a parameter named like the function it belongs to, like protobuf's `std::string u64(uint64_t u64)`, now has its own type. Of two overloads alike but for a `const`, like protobuf's `DynamicCastMessage(const MessageLite* from)` and `DynamicCastMessage(MessageLite* from)`, a call now reaches the one C++ picks: the non-const overload for a pointer to a non-const object, or for the object itself, and the const one for a const object or a temporary, where before the overload declared first won. An argument declared with `const` in front, like `const Buffer* b`, counts as const, and an array as a pointer to its element, so facebook/rocksdb's `Add(level, file, smallest, largest, …)` on `char` arrays reaches the overload taking `const char*`. An object is no longer matched to a pointer parameter, nor a character array to a pointer of another type, so protobuf's `printer->WithVars(EnumVars(…))` reaches `WithVars(Map&& vars)`, not `WithVars(const Map* vars)`. Re-index C++ projects after upgrading. - In C, a call now links to the function of that name its own file can see: its own, else one a header it includes defines, like a `static inline` helper, else one another source file defines without `static`. Before, the function nearest the call was taken, and when that was another source file's `static` function, which no other file can call, the call linked to nothing: in redis, the calls to `server.c`'s `mstime()` and `ustime()` from the rest of the server were lost to the `static` copies other files keep for themselves, and jemalloc's tests' calls to the `do_arena_create` their shared header defines to a neighbouring test's own. A call to a `static inline` function that several headers define reaches the copy in a header the file includes, so protocolbuffers/protobuf's upb code no longer links to the copies in its PHP extension's combined `php-upb.h`. A source file that `#include`s another, like redis's `ae.c` with its event-loop backend or hiredis's `async.c` with `dict.c`, now calls that file's `static` functions, and a header a file includes through an include directory, like jemalloc's `#include "jemalloc/internal/bit_util.h"`, counts as included. A function a header defines only for a build that turns on a flag, like lua-cjson's `fpconv_init` under `#ifdef USE_INTERNAL_FPCONV`, gives way to another source file's definition when nothing in the project's code or build files sets that flag, so redis's calls to `fpconv_init()` reach the `fpconv.c` its build compiles. Re-index C projects after upgrading. - In C++, text inside a string literal is no longer taken for a variable's declaration. That covers the C code protocolbuffers/protobuf's upb generator writes in a raw string (`R"cc( … upb_MiniTableField field = $field_init$; … )cc"`) and log messages like facebook/rocksdb's `"… in WriteBatch handler; …"`. Before, a later call like `field.ctype()` on the function's own `upb::FieldDefPtr field` parameter, or `handler->PutCF(…)` on a `WriteBatch::Handler* handler`, was matched against the type that text seemed to name, so it linked to nothing or to another class's method. Re-index C++ projects after upgrading. - In C++, a call on an `auto` variable now links to the method of the class C++ finds for the type its initializer gives it, as a call on a variable declared with that type does. The type is read from a cast or smart pointer written out, like facebook/rocksdb's `auto iter = std::unique_ptr(…)` or `auto* mutex = static_cast(arg)`, from `new` or `std::make_unique` of a class, and from what a function or method is declared to return, like the `Iterator*` of `txn->GetIterator(…)`, read to the end of an initializer that goes on to the next line. Before, such a variable took the type of an earlier variable of the same name, or none: in RocksDB, `iter->SeekToFirst()` reached `MemTableRep::Iterator` and `mutex->Lock()` reached `port::Mutex`. A chained call like `cfd->current()->storage_info()` now looks up the class the inner call is declared to return the same way, through a smart pointer too, so protocolbuffers/protobuf's Java generator no longer reaches the C# generator's `EnumGenerator`. A type from outside the project no longer links to a class of the project that shares its name: protobuf's `status.ok()` on an `absl::Status` reached `upb::Status::ok`, and fmtlib/fmt's `os.flush()` on a `std::ostream&` reached fmt's own `ostream::flush`. The same holds for a variable declared in a macro body, or typed by what a function in another file returns, and for a class the index doesn't hold whose name only another class shares: protobuf's `_status.ok()` in its `RETURN_IF_ERROR` macro reached `upb::Status::ok`, and `range.options()` on its generated `DescriptorProto::ExtensionRange` reached `Descriptor::ExtensionRange::options`. A namespace a macro stands for is the project's under either name: facebook/rocksdb declares its classes in `namespace ROCKSDB_NAMESPACE` with `#define ROCKSDB_NAMESPACE rocksdb`, so `db->Put(…)` on a `rocksdb::DB*` links to RocksDB's own `DB::Put`. Re-index C++ projects after upgrading. diff --git a/__tests__/cpp-overload-constness.test.ts b/__tests__/cpp-overload-constness.test.ts new file mode 100644 index 00000000..d8d2b11c --- /dev/null +++ b/__tests__/cpp-overload-constness.test.ts @@ -0,0 +1,195 @@ +/** + * A C++ call whose argument fits a function's const and non-const overloads + * alike — protobuf's `DynamicCastMessage(ptr.get())` beside `const T* + * DynamicCastMessage(const MessageLite*)` and `T* DynamicCastMessage(MessageLite*)` + * — reaches the one C++ picks: the non-const overload for a non-const object, + * the const one for a const object or for a value a call returns. Before, the + * overload declared first won the tie. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import type { Node } from '../src/types'; + +const lines = (...l: string[]): string => [...l, ''].join('\n'); + +describe("a C++ call picks between const and non-const overloads by its argument's constness", () => { + let root: string; + let cg: CodeGraph; + + beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cpp-overload-constness-')); + const write = (file: string, text: string): void => { + fs.mkdirSync(path.dirname(path.join(root, file)), { recursive: true }); + fs.writeFileSync(path.join(root, file), text); + }; + // The const overloads come first, as in protobuf's message_lite.h. + write('src/message_lite.h', lines( + '#include ', + 'class MessageLite {', + ' public:', + ' virtual ~MessageLite() {}', + '};', + 'class Message : public MessageLite {};', + 'template ', + 'const T* DynamicCastMessage(const MessageLite* from) { return nullptr; }', + 'template ', + 'T* DynamicCastMessage(MessageLite* from) { return nullptr; }', + 'template ', + 'const T& DynamicCastMessage(const MessageLite& from) { return *static_cast(&from); }', + 'template ', + 'T& DynamicCastMessage(MessageLite& from) { return *static_cast(&from); }', + 'class Buffer {};', + 'class Writer {', + ' public:', + ' void Write(const Buffer* buffer) {}', + ' void Write(Buffer* buffer) {}', + '};', + 'Message MakeMessage() { return Message(); }', + // The non-const overload first. + 'class Reader {', + ' public:', + ' void Read(Buffer* buffer) {}', + ' void Read(const Buffer* buffer) {}', + '};', + // Overloads that differ by more than a `const`, as fmt's `to_string` do. + 'template ', + 'int to_text(T value) { return 0; }', + 'template ', + 'int to_text(const T& value) { return 1; }', + // An array is a pointer to its element, as fmt's `Char buf[8]`. + 'template ', + 'OutputIt format_hex(OutputIt out, unsigned value) { return out; }', + 'template ', + 'Char* format_hex(Char* out, unsigned value) { return out; }', + // rocksdb's static_cast_with_check: the template's own `Src` takes the argument's `const`. + 'template ', + 'Dest* checked_cast(Src* x) { return nullptr; }', + 'template ', + 'std::shared_ptr checked_cast(std::shared_ptr&& x) { return nullptr; }', + // fmt's to_string_view: an argument of a template parameter's type could be a pointer. + 'template ', + 'int view_of(const T* s) { return 0; }', + 'template ', + 'int view_of(const T& s) { return 1; }', + // rocksdb's Put: characters are no ColumnFamilyHandle. + 'class Handle {};', + 'class Slice {', + ' public:', + ' Slice(const char* data) {}', + '};', + 'class Store {', + ' public:', + ' int Put(Handle* handle, const Slice& key) { return 0; }', + ' int Put(const Slice& key, const Slice& value) { return 1; }', + '};', + // ...but a template's own `T*` takes them. + 'template ', + 'int sink(T* p) { return 0; }', + 'int sink(const Slice& s) { return 1; }', + // A pointer is no characters, a `constexpr` array is const, and a long header is still a header. + 'int Emit(const char* text) { return 0; }', + 'int Emit(const Slice& text) { return 1; }', + 'int Take(char* p) { return 0; }', + 'int Take(const char* p) { return 1; }', + 'template ', + 'int peek(T* p) { return 0; }', + 'int peek(const Slice& s) { return 1; }', + )); + write('src/use.cc', lines( + '#include "src/message_lite.h"', + 'void NonConstHolder(std::shared_ptr ptr) { DynamicCastMessage(ptr.get()); }', + 'void ConstHolder(std::shared_ptr ptr) { DynamicCastMessage(ptr.get()); }', + 'void NonConstPointer(Message* m) { DynamicCastMessage(m); }', + 'void ConstPointer(const Message* m) { DynamicCastMessage(m); }', + 'void NonConstReference(Message& m) { DynamicCastMessage(m); }', + 'void ConstReference(const Message& m) { DynamicCastMessage(m); }', + 'void Temporary() { DynamicCastMessage(MakeMessage()); }', + 'void WriteNonConst(Writer& w, Buffer* b) { w.Write(b); }', + 'void WriteConst(Writer& w, const Buffer* b) { w.Write(b); }', + 'void WriteConstPointer(Writer& w, Buffer* const b) { w.Write(b); }', + 'void ReadNonConst(Reader& r, Buffer* b) { r.Read(b); }', + 'void ReadConst(Reader& r, const Buffer* b) { r.Read(b); }', + 'void Show(Buffer& b) { to_text(b); }', + 'template ', + 'void WriteHex(unsigned v) { Char buf[8]; format_hex(buf, v); }', + 'void Merge(const Buffer& other) { checked_cast(&other); }', + 'template ', + 'void Format(const S& fmt) { view_of(fmt); }', + // A header over several lines, as fmt's `join` writes. + 'template ', + 'void Join(It begin, Sentinel end, S&& sep) { view_of(sep); }', + 'void Save(Store& s) {', + ' char key[] = "key";', + ' char value[] = "value";', + ' s.Put(key, value);', + '}', + 'void Drain() { char buf[4]; sink(buf); }', + 'void Send(Buffer* b) { Emit(b); }', + 'void Keep() {', + ' constexpr char name[] = "name";', + ' Take(name);', + '}', + 'void Look(const Buffer* b) { peek(b); }', + )); + cg = await CodeGraph.init(root, { index: true }); + }, 120_000); + + afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); + }); + + /** The `name` overloads a function calls, as `name:startLine`. */ + const calls = (caller: string, name: string): string[] => { + const fn = cg.getNodesByKind('function').find((n: Node) => n.name === caller); + expect(fn, caller).toBeDefined(); + return cg.getCallees(fn!.id).filter((r) => r.edge.kind === 'calls' && r.node.name === name).map((r) => `${r.node.name}:${r.node.startLine}`); + }; + + it('a pointer to a non-const object takes the non-const overload, to a const one the const overload', () => { + expect(calls('NonConstHolder', 'DynamicCastMessage')).toEqual(['DynamicCastMessage:10']); + expect(calls('ConstHolder', 'DynamicCastMessage')).toEqual(['DynamicCastMessage:8']); + expect(calls('NonConstPointer', 'DynamicCastMessage')).toEqual(['DynamicCastMessage:10']); + expect(calls('ConstPointer', 'DynamicCastMessage')).toEqual(['DynamicCastMessage:8']); + }); + + it('a non-const object takes the non-const reference, a const object or a temporary the const one', () => { + expect(calls('NonConstReference', 'DynamicCastMessage')).toEqual(['DynamicCastMessage:14']); + expect(calls('ConstReference', 'DynamicCastMessage')).toEqual(['DynamicCastMessage:12']); + expect(calls('Temporary', 'DynamicCastMessage')).toEqual(['DynamicCastMessage:12']); + }); + + it("a class's own overloads, whichever comes first", () => { + expect(calls('WriteNonConst', 'Write')).toEqual(['Write:19']); + expect(calls('WriteConst', 'Write')).toEqual(['Write:18']); + // A pointer's own `const` is not its object's. + expect(calls('WriteConstPointer', 'Write')).toEqual(['Write:19']); + expect(calls('ReadNonConst', 'Read')).toEqual(['Read:24']); + expect(calls('ReadConst', 'Read')).toEqual(['Read:25']); + }); + + it('no overload that differs by more than a `const`, and an array as a pointer', () => { + expect(calls('Show', 'to_text')).toEqual(['to_text:28']); + expect(calls('WriteHex', 'format_hex')).toEqual(['format_hex:34']); + }); + + it("a template's own parameter, an argument of a template parameter's type, and characters for a class pointer", () => { + expect(calls('Merge', 'checked_cast')).toEqual(['checked_cast:36']); + expect(calls('Format', 'view_of')).toEqual(['view_of:40']); + expect(calls('Join', 'view_of')).toEqual(['view_of:40']); + expect(calls('Save', 'Put')).toEqual(['Put:51']); + expect(calls('Drain', 'sink')).toEqual(['sink:54']); + expect(calls('Send', 'Emit')).toEqual(['Emit:57']); + expect(calls('Keep', 'Take')).toEqual(['Take:59']); + expect(calls('Look', 'peek')).toEqual(['peek:64']); + }); +}); diff --git a/src/resolution/cpp-type-aliases.ts b/src/resolution/cpp-type-aliases.ts index 49b341b9..740da793 100644 --- a/src/resolution/cpp-type-aliases.ts +++ b/src/resolution/cpp-type-aliases.ts @@ -375,7 +375,8 @@ export function clearCppTypeAliasMemos(context: ResolutionContext): void { /** Add the parameter names of the `template <…>` header that ends right where a declaration starts. */ function collectTemplateHeader(lines: readonly string[], startLine: number, startColumn: number, names: Set): void { - const before = [...lines.slice(Math.max(0, startLine - 4), startLine - 1), (lines[startLine - 1] ?? '').slice(0, startColumn)].join('\n'); + // (A header can take several lines: fmt's `join` spends four on its parameters' defaults.) + const before = [...lines.slice(Math.max(0, startLine - 9), startLine - 1), (lines[startLine - 1] ?? '').slice(0, startColumn)].join('\n'); const at = before.lastIndexOf('template'); // Specifiers, attributes and macros may come between the header and where // the declaration starts: protobuf's `template cppCodeOf(line) ?? '').join(' ').trimEnd(); let depth = 0; for (let i = before.endsWith('>') ? before.length - 1 : -1; i >= 0; i--) { @@ -9920,10 +9920,11 @@ const CPP_LIBRARY_ADAPTORS: ReadonlySet = new Set(['stack', 'queue', 'pr */ function cppLibraryElementOf(object: CppWrittenType, member: string, at: UnresolvedRef, context: ResolutionContext): CppWrittenType | null { if (isCppPointerType(object.text)) return null; - const text = cppWithoutTypeKeywords(object.text); - const segments = cppTypeSegments(text); + const segments = cppTypeSegments(cppWithoutTypeKeywords(object.text)); const last = segments?.[segments.length - 1]; if (!segments || !last) return null; + // (Its own `const` and `&` aside: a `const` in a template argument, `std::shared_ptr`, stays.) + const text = object.text.replace(/^\s*(?:typename\s+)?const\s+/, '').replace(/(?:\s*\bconst\b)?\s*&+\s*$/, '').trim(); const library = segments[0] === 'std' || segments[0] === 'absl'; if (member === 'front' || member === 'back' || member === 'at') { if ((library && CPP_LIBRARY_SEQUENCES.has(last)) || CPP_PROJECT_SEQUENCES.has(last)) return cppRangeElementOf(object, at, context); @@ -10270,6 +10271,8 @@ interface CppReceiverDeclaration { classTemplate?: boolean; /** The declared type as written (`std::vector*`). */ raw?: string; + /** A `const` the declaration writes before its type (`const Buffer* b`), which `raw` leaves out. */ + constant?: boolean; /** The array dimensions the declarator gives the receiver (`Status statuses[3]`), which `raw` doesn't show. */ array?: number; /** The declaration is the calling function's own, or a member of its class (isCppCallersDeclaration). */ @@ -11232,22 +11235,65 @@ function cppOverloadFor(candidates: Node[], name: string, ref: UnresolvedRef, co /** * Of overloads the call's arguments `args` fit alike by count (in * preference order), the one the types the arguments have where the call - * reads them fit best (cppArgumentTypeFit); the first on a tie. + * reads them fit best (cppArgumentTypeFit); the first on a tie, unless a + * const twin of it fits the arguments' constness better (cppConstTwinChoice). */ function cppArgumentTypeChoice(tied: Node[], name: string, args: string[], ref: UnresolvedRef, context: ResolutionContext): Node { if (tied.length === 1 || args.length === 0) return tied[0]!; const types = args.map((arg) => cppArgumentType(arg, ref, context)); if (types.every((t) => t === null)) return tied[0]!; - let best = tied[0]!; - let bestFit = cppArgumentTypeFit(best, name, types, context); - for (const n of tied.slice(1)) { - const fit = cppArgumentTypeFit(n, name, types, context); - if (fit > bestFit) { - best = n; - bestFit = fit; + const fits = tied.map((n) => cppArgumentTypeFit(n, name, types, context)); + const top = Math.max(...fits); + const best = tied[fits.indexOf(top)]!; + return cppConstTwinChoice(best, tied.filter((n, i) => fits[i] === top && n !== best), name, types, context); +} + +/** + * `best`, or of its twins among `others` — overloads alike but for the + * `const` of a pointer or reference parameter — the one that adds `const` to + * the fewest of the arguments `types`, as C++ ranks them: protobuf's + * `DynamicCastMessage(ptr.get())` on a `std::shared_ptr ptr` is + * `T* DynamicCastMessage(MessageLite* from)`, not the `const T* + * DynamicCastMessage(const MessageLite* from)` declared before it. (A const + * object fits only the const twin: cppWrittenTypeFit.) + */ +function cppConstTwinChoice(best: Node, others: Node[], name: string, types: Array, context: ResolutionContext): Node { + if (others.length === 0) return best; + const shape = cppConstlessParameters(best, name, context)?.join(','); + if (shape === undefined) return best; + let choice = best; + let added = cppConstAdded(best, name, types, context); + for (const n of others) { + if (cppConstlessParameters(n, name, context)?.join(',') !== shape) continue; + const count = cppConstAdded(n, name, types, context); + if (count < added) { + choice = n; + added = count; } } - return best; + return choice; +} + +/** An overload's parameter types without their `const`s: what its const twins share. */ +function cppConstlessParameters(n: Node, name: string, context: ResolutionContext): string[] | null { + const list = cppOverloadParameters(n, name, context); + if (list === null) return null; + return splitCppTopLevel(cppWithoutComments(list)).filter((p) => p !== 'void') + .map((p) => cppParameterType(p).replace(/\bconst\b/g, ' ').replace(/\s+/g, '')); +} + +/** How many of the arguments `types` an overload's pointer or reference parameters add `const` to. */ +function cppConstAdded(n: Node, name: string, types: Array, context: ResolutionContext): number { + const list = cppOverloadParameters(n, name, context); + if (list === null) return 0; + const params = splitCppTopLevel(cppWithoutComments(list)).filter((p) => p !== 'void'); + let count = 0; + for (let i = 0; i < types.length && i < params.length; i++) { + const arg = types[i]; + const param = cppParameterType(params[i]!); + if (arg?.kind === 'type' && cppRefersToConst(param) === true && cppArgumentConst(arg.text, param) === false) count++; + } + return count; } /** @@ -11415,7 +11461,9 @@ function isCppDeletedFunction(n: Node, name: string, context: ResolutionContext) } /** What a C++ call's argument is: a literal's kind, or the type a name or expression has as written. */ -type CppArgumentType = { kind: 'char' | 'number' | 'zero' | 'bool' | 'null' } | { kind: 'type'; text: string }; +type CppArgumentType = + | { kind: 'char' | 'number' | 'zero' | 'bool' | 'null' } + | { kind: 'type'; text: string; lvalue?: boolean; dependent?: boolean }; /** * What the argument `arg` of the call at `ref` is, where that can be read: a @@ -11437,12 +11485,43 @@ function cppArgumentType(arg: string, ref: UnresolvedRef, context: ResolutionCon if (name) { const decl: CppReceiverDeclaration = {}; inferCppReceiverType(name, ref, context, 0, decl); - const written = decl.declared?.text ?? decl.raw; - if (!written || !decl.callers || decl.shadowed) return null; - return { kind: 'type', text: address ? `${written}*` : written }; + const declared = decl.declared?.text ?? decl.raw; + if (!declared || !decl.callers || decl.shadowed) return null; + const written = decl.constant && !/^\s*const\b/.test(declared) ? `const ${declared}` : declared; + // (An array is a pointer to its element: fmt's `format_base2e(4, buf, …)` on a `Char buf[8]` is the `Char* out` overload.) + const typed = address || decl.array ? `${written.replace(/\s*&+\s*$/, '')}*` : written; + return { kind: 'type', text: typed, lvalue: !address, dependent: isCppDependentArgument(typed, ref, context) }; } const typed = /^(?:u8|u|U|L)?R?["']/.test(text) ? null : cppExpressionType(text, ref, context, 0); - return typed ? { kind: 'type', text: typed.text } : null; + return typed + ? { kind: 'type', text: typed.text, lvalue: cppNamesObject(text, typed.text), dependent: isCppDependentArgument(typed.text, ref, context) } + : null; +} + +/** + * Whether an argument of the written type `type` has the type of a template + * parameter of the calling function or its class (fmt's `const S& fmt`), + * which could be a pointer as well as an object. + */ +function isCppDependentArgument(type: string, ref: UnresolvedRef, context: ResolutionContext): boolean { + const name = normalizeCppTypeName(type); + const caller = name ? context.getNodeById?.(ref.fromNodeId) : null; + return !!caller && cppTemplateParameters(caller, context).has(name!); +} + +/** + * Whether the C++ argument `expr`, of the written type `type`, names an + * object, which a reference to a non-const type can bind: a dereference, a + * subscript, a member, or a call that returns an lvalue reference. False for + * a call that returns a value; undefined when that can't be told. + */ +function cppNamesObject(expr: string, type: string): boolean | undefined { + let text = expr.trim(); + while (text.startsWith('(') && text.endsWith(')') && cppOpeningParen(text, text.length - 1) === 0) text = text.slice(1, -1).trim(); + if (text.startsWith('*') || text.endsWith(']')) return true; + if (!text.endsWith(')')) return /^[A-Za-z_][\w\s.:>-]*$/.test(text) ? true : undefined; + if (/&&\s*$/.test(type)) return undefined; + return /&\s*$/.test(type); } /** What a C++ type is, for an argument to fit a parameter of it. */ @@ -11482,13 +11561,18 @@ function cppArgumentTypeFit(n: Node, name: string, types: Array p !== 'void'); + const own = types.some((t) => t?.kind === 'type') ? cppOwnTemplateParameters(n, context) : []; let score = 0; for (let i = 0; i < types.length && i < params.length; i++) { const arg = types[i]; const param = cppParameterType(params[i]!); if (!arg || param.includes('...')) continue; const category = cppTypeCategory(param); - score += arg.kind === 'type' ? cppWrittenTypeFit(arg.text, param, category) : CPP_LITERAL_FIT[arg.kind][category]; + // (A parameter of the function's own template parameter's type, `T* x`, takes the argument's `const` into `T`.) + const deduced = own.length > 0 && own.includes(normalizeCppTypeName(param) ?? ''); + score += arg.kind === 'type' + ? cppWrittenTypeFit(arg.text, param, category, { lvalue: arg.lvalue, dependent: arg.dependent, deduced }) + : CPP_LITERAL_FIT[arg.kind][category]; } return score; } @@ -11506,8 +11590,66 @@ const CPP_LITERAL_FIT: Record<'char' | 'number' | 'zero' | 'bool' | 'null', Reco null: { null: 4, pointer: 3, cstring: 3, class: 1, string: -3, char: -3, number: -3, bool: -1 }, }; -/** How an argument written as type `arg` fits a parameter of type `param` (of category `category`). */ -function cppWrittenTypeFit(arg: string, param: string, category: CppTypeCategory): number { +/** + * How an argument written as type `arg` fits a parameter of type `param` (of + * category `category`). An object is no pointer, nor a pointer to characters + * a pointer to anything but characters or `void`, or the other way round: + * protobuf's `DynamicCastMessage(*msg)` takes a reference, and rocksdb's + * `db_->Put(opts, key, ts, value)` on a `char key[]` no `ColumnFamilyHandle*`. + * A pointer or reference to a non-const type never takes a const object, + * nor a reference a value a call returns (`lvalue` false). None of that + * holds for an argument of a template parameter's type (`dependent`), and + * the pointer and `const` rules not for a parameter whose type the function's + * own template parameter is (`deduced`). (Of two overloads alike but for a + * `const`, which one a non-const object takes is cppConstTwinChoice's.) + */ +function cppWrittenTypeFit( + arg: string, + param: string, + category: CppTypeCategory, + how: { lvalue?: boolean; dependent?: boolean; deduced?: boolean } = {}, +): number { + const fit = cppUnqualifiedTypeFit(arg, param, category); + if (fit < 0 || how.dependent) return fit; + const argCategory = cppTypeCategory(arg); + if (category === 'pointer' && argCategory === 'class') return -3; + if (!how.deduced && category === 'pointer' && argCategory === 'cstring' && !/\bvoid\b/.test(param)) return -3; + if (category === 'cstring' && argCategory === 'pointer') return -3; + if (cppRefersToConst(param) !== false) return fit; + if (!how.deduced && cppArgumentConst(arg, param) === true) return -3; + return how.lvalue === false && !/\*/.test(stripCppTemplateArguments(param)) ? -3 : fit; +} + +/** + * Whether the object an argument of the written type `arg` gives a pointer + * or reference parameter `param` is const: what the pointer points at, or + * the object itself. Undefined when the argument is no such thing. + */ +function cppArgumentConst(arg: string, param: string): boolean | undefined { + const argPointer = /\*/.test(stripCppTemplateArguments(arg)); + if (/\*/.test(stripCppTemplateArguments(param))) return argPointer ? cppRefersToConst(arg) : undefined; + return argPointer ? undefined : cppIsConstObject(arg); +} + +/** + * Whether the C++ type `type`, a pointer or an lvalue reference of one + * level, refers to a const object: `const T*`, `T const&`. (A pointer's own + * `const`, `T* const`, is not the object's.) Undefined for any other type. + */ +function cppRefersToConst(type: string): boolean | undefined { + const text = stripCppTemplateArguments(type).trim(); + const stars = (text.match(/\*/g) ?? []).length; + if (stars > 1 || (stars === 0 && !/[^&]&\s*$/.test(text))) return undefined; + return /\bconst\b/.test(stars === 1 ? text.slice(0, text.indexOf('*')) : text); +} + +/** Whether a C++ object of the written type `type`, a value or a reference, is const. */ +function cppIsConstObject(type: string): boolean { + return /\bconst\b/.test(stripCppTemplateArguments(type)); +} + +/** How an argument written as type `arg` fits a parameter of type `param`, `const` aside. */ +function cppUnqualifiedTypeFit(arg: string, param: string, category: CppTypeCategory): number { const argCategory = cppTypeCategory(arg); const pointer = (c: CppTypeCategory): boolean => c === 'pointer' || c === 'cstring'; if (normalizeCppTypeName(arg) === normalizeCppTypeName(param) && pointer(argCategory) === pointer(category)) return 3; @@ -11699,12 +11841,16 @@ function cppRebindsReceiver(line: string, escapedReceiver: string): boolean { } /** Record the C++ declaration receiver inference settles on, with the array dimensions its declarator gives it. */ -function noteCppDeclaration(found: CppReceiverDeclaration | undefined, raw: string, callers: boolean, array = 0): void { +/** A `const` (or `constexpr`) right before a declarator's type: `(Writer& w, const ` of `const Buffer* b`. */ +const CPP_LEADING_CONST = /\b(?:const|constexpr)\s+$/; + +function noteCppDeclaration(found: CppReceiverDeclaration | undefined, raw: string, callers: boolean, array = 0, constant = false): void { if (!found) return; found.raw = raw; found.callers = callers; found.pointer = isCppPointerType(raw); found.array = array > 0 ? array : undefined; + found.constant = constant || undefined; } /** @@ -11791,7 +11937,7 @@ function inferCppReceiverType( } else if (normalized) { const inCallerScope = isCppCallersDeclaration(ref.filePath, i + 1, ref, context); const array = cppArrayDimensions(line.slice(declaratorMatch.index + declaratorMatch[0].length)); - noteCppDeclaration(found, declaratorMatch[1]!, inCallerScope, array); + noteCppDeclaration(found, declaratorMatch[1]!, inCallerScope, array, CPP_LEADING_CONST.test(line.slice(0, declaratorMatch.index))); return cppDeclaredType(declaratorMatch[1]!, normalized, inCallerScope, ref, context, found); } else if (cppTypeBeganAbove(declaratorMatch[1] ?? '')) { // The end of a declaration begun on an earlier line, maybe the @@ -11872,7 +12018,7 @@ function inferCppReceiverType( if (normalized && normalized !== 'auto') { const inCallerScope = isCppCallersDeclaration(headerPath, i + 1, ref, context); const array = cppArrayDimensions(line.slice(declaratorMatch.index + declaratorMatch[0].length)); - noteCppDeclaration(found, declaratorMatch[1]!, inCallerScope, array); + noteCppDeclaration(found, declaratorMatch[1]!, inCallerScope, array, CPP_LEADING_CONST.test(line.slice(0, declaratorMatch.index))); return cppDeclaredType(declaratorMatch[1]!, normalized, inCallerScope, ref, context, found); } }