TSLANG_ISSUES 5: @cpp C++ overloads under one name, picked by the arguments

README.md

@@ -713,7 +713,7 @@ type Handle = [p: Opaque | null];
 class Shape { // geo::Shape
     @slot(1) area(): s32;
     @slot(3) scale(by: s32): void;
-    @slot(2) scale_2(by: f64): void;
+    @slot(2) scale(by: f64): void;
     @cppnontrivial
     @slot(4) handle(): [p: Opaque | null];
     @linkname("?id@Shape@geo@@QEAAHH@Z") id(x: s32): s32;
@@ -727,7 +727,7 @@ declare function make_square(side: s32): Square;
 ```
 
 - **Slots** are clang's: the destructor (slot 0 under MSVC, 0 and 1 under the Itanium ABI) and methods that aren't public are slots left out. A method overriding a base's is not declared again: the `@cpp` class's override keeps the base's slot.
-- **Overloads** get `_2`, `_3`, ... in declaration order, whatever order the ABI puts their slots in.
+- **Overloads** keep the C++ name, each with its own slot (or symbol), in whatever order the ABI puts the slots: a call picks the overload by its arguments, as C++ does (`shape.scale(2)`, `shape.scale(0.5)`). A method named as a field gets `_2`, `_3`, ...
 - **A pointer or a reference** to an object of a bound class is the class (`| null` for a pointer); a reference to a struct is a `Reference` to its tuple.
 - **A record returned by value** is a tuple with its layout, `@cppnontrivial` when it has a destructor or a copy constructor.
 - **Fields** are declared when they are where a `@cpp` class puts them (after the vtable pointer and the base's storage).

TSLANG_ISSUES.md

@@ -13,7 +13,7 @@ https://github.com/ASDAlexander77/native_ts_graphics
 | 2 | The error for (1) pointed at line 1, column 1 of the base class | bug | **moot**: nothing synthesized fails any more; an explicit bad `super()` is reported at the call |
 | 3 | `u64` `<<` shifted by the count modulo 32 | bug | **fixed** |
 | 4 | `Ref<string>` field with a `null` initializer rejected | question | **answered**: declare it `Ref<string> \| null` |
-| 5 | `@cppvtable` overload naming (C++ overloads need distinct TypeScript names) | open | **addressed** on branch `cppvtable-slot`: `@cpp` + `@slot(N)` (docs/cpp-classes.md) |
+| 5 | `@cppvtable` (now `@cpp`) overload naming (C++ overloads need distinct TypeScript names) | open | **fixed**: `@slot(N)` (#561), slots from tsbindgen (#562), and overloads under one name, picked by the arguments (this PR) |
 | 6 | C `string` return values keep the raw `char*` (no copy) | open | **fixed**: copied at the call (this PR; `Opaque` is the raw pointer) |
 | 7 | `Opaque` compared with `Opaque \| null` crashed the compiler | - | **fixed** by #536 (re-checked with the original drawScene shape) |
 | 8 | A ternary of number literals in arithmetic truncated to the other operand's integer type | - | **fixed** by #560 (still wrong on pre-alpha97) |
@@ -112,6 +112,12 @@ interface method need distinct TypeScript names, and the MSVC slot order of over
 hand (`cl /d1reportSingleClassLayout<Class>`). A way to name the C++ overload (or a tool that prints the
 slot map) would remove that manual step. See `docs/cppvtable-classes.md`. Not re-checked.
 
+**Fixed**: `@cppvtable` is now `@cpp`, and each virtual method names its slot with `@slot(N)` (#561), which
+tsbindgen takes from clang (#562), MSVC's reversed overload order included. C++ overloads are now declared
+under the C++ name, each with its own `@slot` or `@linkname`, and a call picks one by its arguments as C++
+does (`methods.over(5)`, `methods.over(2.5)`); no fit, or two equal fits, is an error listing the
+overloads. tsbindgen no longer writes `_2`, `_3` (docs/cpp-classes.md).
+
 ### 6. C `string` return values are not copied
 
 A C/C++ function declared as returning `string` hands tslang its raw `char*`, with no copy and no

docs/cpp-classes.md

@@ -100,9 +100,20 @@ Writing one by hand:
   (`<name>.<os>.ts`).
 - **The destructor** is one slot under MSVC (the scalar deleting destructor) and two under the
   Itanium ABI. Don't call it: release objects the way the C++ library does (`Release()`).
-- **Overloads.** tslang methods are identified by name, so every C++ overload needs its own
-  TypeScript name, with its own `@slot`. MSVC groups overloads in the vtable, and not always in
-  declaration order: take the numbers from the layout report.
+- **Overloads.** Declare each C++ overload under the C++ name, with its own `@slot` (or
+  `@linkname` for a non-virtual one). A call picks the overload from its arguments, at compile
+  time as C++ does: an argument of the parameter's own type fits best, then an integer for an
+  integer or a float for a float, then any other conversion. A call that fits none, or fits two
+  as well, is an error that lists the overloads. MSVC groups overloads in the vtable, in reverse
+  declaration order: take the numbers from the layout report (tsbindgen does).
+
+  ```ts
+  @slot(8) over(x: s32): s32;
+  @slot(7) over(x: f64): s32;
+  @slot(6) over(a: s32, b: s32): s32;   // methods.over(5), methods.over(2.5), methods.over(6, 7)
+  ```
+
+  A derived class overriding one overload names its slot: by name alone it would be ambiguous.
 - **Non-virtual methods** are bound by their mangled symbol: `@linkname("?scaled@Methods@@QEAAHH@Z")`
   under MSVC, `@linkname("_ZN7Methods6scaledEi")` under the Itanium ABI. The symbol has to be
   exported by the C++ library (`__declspec(dllexport)` on the method or the class).

tslang/include/TsClangImporter/BindingModel.h

@@ -88,7 +88,7 @@ enum class DeclKind
 // A method of a C++ class bound as a @cpp class.
 struct Method
 {
-    std::string name;        // the TS name: the C++ name, `_2`, `_3`, ... for later overloads
+    std::string name;        // the TS name: the C++ name (an overload too), `_2`, ... when a field has it
     std::string cppName;     // the C++ name, for messages
     int slot = -1;           // a virtual method's vtable slot; -1 for a non-virtual one
     std::string symbol;      // a non-virtual method's mangled symbol

tslang/include/TypeScript/Defines.h

@@ -202,6 +202,8 @@
 // @slot(N) on a method of a @cpp class: a C++ virtual method, slot N of the vtable (counted
 // from 0, a base class's slots included); a method without it isn't virtual (see MethodInfo::cppSlot)
 #define CPP_SLOT "slot"
+// The name of a C++ overload after the first one declared, `write#1` (see mlirGenClassCppOverloadNames)
+#define CPP_OVERLOAD_SEPARATOR "#"
 // On a call (CallIndirect, then CallInternal or Invoke): the index of the argument that is the hidden
 // pointer the callee returns a record through, which LowerToLLVM marks `sret` for the target to pass
 // it where the C++ ABI does (X8 on AArch64)

tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h

@@ -983,7 +983,8 @@ struct ClassInfo
                 auto index = it->second;
                 if (isCppVTable && method.cppSlot >= 0 && (size_t)method.cppSlot != index && cppSlotConflict.empty())
                 {
-                    cppSlotConflict = "'" + method.name + "' overrides slot " + std::to_string(index) + " of a base class, not slot "
+                    cppSlotConflict = "'" + mlir::StringRef(method.name).split(CPP_OVERLOAD_SEPARATOR).first.str() + "' overrides slot "
+                        + std::to_string(index) + " of a base class, not slot "
                         + std::to_string(method.cppSlot);
                 }
 
@@ -998,6 +999,23 @@ struct ClassInfo
                 // @slot(N): slot N, whatever the declaration order; the slots no method names
                 // are gaps, which a class implementing C++ methods leaves null
                 auto index = (size_t)method.cppSlot;
+
+                // a C++ overload of a base class's method takes other parameters: with the same ones
+                // it is that method, overridden in the wrong slot
+                auto declaredName = mlir::StringRef(method.name).split(CPP_OVERLOAD_SEPARATOR).first;
+                auto params = method.funcType.getInputs().drop_front(method.funcType.getNumInputs() > 0 ? 1 : 0);
+                for (auto [slot, entry] : llvm::enumerate(vtable))
+                {
+                    auto &inherited = entry.methodInfo;
+                    if (!entry.isInterfaceVTable && !entry.isStaticField && !inherited.isCppSlotGap && slot != index
+                        && inherited.funcType && inherited.funcType.getNumInputs() > 0
+                        && mlir::StringRef(inherited.name).split(CPP_OVERLOAD_SEPARATOR).first == declaredName
+                        && inherited.funcType.getInputs().drop_front() == params && cppSlotConflict.empty())
+                    {
+                        cppSlotConflict = "'" + declaredName.str() + "' overrides slot " + std::to_string(slot)
+                            + " of a base class, not slot " + std::to_string(index);
+                    }
+                }
                 while (vtable.size() <= index)
                 {
                     MethodInfo gap;
@@ -1017,7 +1035,8 @@ struct ClassInfo
                 {
                     if (cppSlotConflict.empty())
                     {
-                        cppSlotConflict = "'" + method.name + "' and '" + vtable[index].methodInfo.name + "' are both in slot "
+                        cppSlotConflict = "'" + mlir::StringRef(method.name).split(CPP_OVERLOAD_SEPARATOR).first.str() + "' and '"
+                            + mlir::StringRef(vtable[index].methodInfo.name).split(CPP_OVERLOAD_SEPARATOR).first.str() + "' are both in slot "
                             + std::to_string(index);
                     }
 

tslang/lib/TsClangImporter/HeaderParser.cpp

@@ -513,13 +513,17 @@ class Collector : public clang::RecursiveASTVisitor<Collector>
         }
 
         llvm::StringSet<> inherited;
+        llvm::StringSet<> inheritedMethods;
         if (base)
         {
             inherited = classNames[decl.base];
+            inheritedMethods = classMethodNames[decl.base];
         }
 
         auto &usedNames = classNames[key];
         usedNames = std::move(inherited);
+        auto &methodNames = classMethodNames[key];
+        methodNames = std::move(inheritedMethods);
 
         addClassFields(decl, definition, base);
         for (auto &field : decl.fields)
@@ -538,15 +542,18 @@ class Collector : public clang::RecursiveASTVisitor<Collector>
                 continue;
             }
 
+            // C++ overloads keep the C++ name, each with its own @slot or @linkname: tslang picks one
+            // by the arguments. A field of the name makes it `_2`, `_3`, ...
             Method bound;
             bound.cppName = method->getName().str();
             bound.name = escapeReserved(bound.cppName);
-            for (auto overload = 2; usedNames.count(bound.name); ++overload)
+            for (auto suffix = 2; usedNames.count(bound.name) && !methodNames.count(bound.name); ++suffix)
             {
-                bound.name = escapeReserved(bound.cppName) + "_" + std::to_string(overload);
+                bound.name = escapeReserved(bound.cppName) + "_" + std::to_string(suffix);
             }
 
             usedNames.insert(bound.name);
+            methodNames.insert(bound.name);
             mapMethod(bound, method);
             decl.methods.push_back(std::move(bound));
         }
@@ -755,8 +762,10 @@ class Collector : public clang::RecursiveASTVisitor<Collector>
     HeaderModel &model;
     llvm::StringSet<> keys;
     std::vector<std::pair<size_t, const clang::RecordDecl *>> pendingRecords;
-    // a @cpp class's TS member names, its base's included: a later overload gets `_2`, `_3`, ...
+    // a @cpp class's TS member names, its base's included: a method named as a field gets `_2`, ...
     llvm::StringMap<llvm::StringSet<>> classNames;
+    // those that are methods: an overload keeps the name
+    llvm::StringMap<llvm::StringSet<>> classMethodNames;
     // where a @cpp class's declared fields end, padded to its alignment, and that alignment (bits):
     // a derived class's fields follow
     llvm::DenseMap<const clang::CXXRecordDecl *, std::pair<uint64_t, uint64_t>> fieldsEnd;

tslang/lib/TypeScript/DeclarationPrinter.cpp

@@ -680,9 +680,11 @@ namespace typescript
 
             // a @cpp method returning a record: as declared, the importer lowers it again
             auto funcType = method.declaredFuncType ? method.declaredFuncType : method.funcType;
+            // a C++ overload after the first is `name#N` here (mlirGenClassCppOverloadNames); the
+            // importer renames it again
             printMethod(
                 method.isStatic,
-                method.name,
+                StringRef(method.name).split(CPP_OVERLOAD_SEPARATOR).first,
                 funcType.getParams(),
                 funcType.getNumResults() > 0 ? funcType.getResult(0) : mlir::Type(),
                 classType->classType);