From 3ea39df16a203d658534fb88930ac3e60d020e45 Mon Sep 17 00:00:00 2001 From: Morten Borup Petersen Date: Sat, 7 Sep 2024 10:00:54 +0200 Subject: [PATCH] Don't predeclare a struct as a class (#114) Holy moly this took a long time to debug. MSVC gives absolutely zero warnings about if a struct/class is predeclared as one or the other, during compilation. However, linking also needs to happen. And as it turns out, [whether a thing is a class or struct is part of the MSVC name mangling spec](https://en.wikiversity.org/wiki/Visual_C%2B%2B_name_mangling). Hence, we'd end up with e.g. this function (which references `SymbolInfo`) as being defined in it's object file as the following symbol: > `lookupSymbolsByLocation@SymbolTable@textplan@substrait@io@@QEBA?AV?$vector@PEBUSymbolInfo@textplan@substrait@io@@V?$allocator@PEBUSymbolInfo@textplan@substrait@io@@@std@@@std@@AEBVLocation@234@@Z` However, things which reference `SymbolInfo`, but which may have pulled in the incorrect predeclaration prior, will reference it as an incorrect symbol: > `lookupSymbolsByLocation@SymbolTable@textplan@substrait@io@@QEBA?AV?$vector@PEBVSymbolInfo@textplan@substrait@io@@V?$allocator@PEBVSymbolInfo@textplan@substrait@io@@@std@@@std@@AEBVLocation@234@@Z ...` e.g. notice the difference between `PEBUSymbolInfo` and `PEBVSymbolInfo`. Enqueue strange linking issues galore. --- src/substrait/textplan/StructuredSymbolData.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/substrait/textplan/StructuredSymbolData.h b/src/substrait/textplan/StructuredSymbolData.h index 98c09cc9..36ceee85 100644 --- a/src/substrait/textplan/StructuredSymbolData.h +++ b/src/substrait/textplan/StructuredSymbolData.h @@ -9,7 +9,7 @@ namespace io::substrait::textplan { -class SymbolInfo; +struct SymbolInfo; // Used by the PlanRelation and Relation concepts to track connectivity. struct RelationData {