From f74073a48604dd974b9830d2e15e6a7c4ab2f4cb Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 15:45:27 +0000 Subject: [PATCH] Write a header's definitions so it can be included from more than one translation unit [patch] A SourceFile marked IsHeader only gained #pragma once, so a free function, a function a method is lowered to, or a mutable file-scope variable in it was defined with external linkage in every translation unit that included it, and the program failed to link with "multiple definition". C++ now writes free functions and namespace-scope variables inline. C writes functions static inline and mutable variables static, one copy per translation unit, since C has no inline variable. Output that is not a header is unchanged. Fixes ktsu-dev/Coder#423 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW4Rc5sqSEVHr6NgrkT --- CLAUDE.md | 7 +- Coder.Test/Languages/HeaderLinkageTests.cs | 234 +++++++++++++++++++++ Coder/Languages/CFamilyGenerator.cs | 12 ++ Coder/Languages/CGenerator.cs | 27 ++- Coder/Languages/CppGenerator.cs | 28 ++- 5 files changed, 298 insertions(+), 10 deletions(-) create mode 100644 Coder.Test/Languages/HeaderLinkageTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 861e124..c42369f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -63,7 +63,12 @@ source in seven target languages. The solution uses: `VariableDeclaration`: a field with no initialiser is value-initialised, so a default-constructed instance is the one the declaration described, and a local with none is ordinary. `SourceFile`'s `Imports` are the one part of the AST that does not translate — a C++ include path, a C# namespace - and a Python module are different kinds of thing — so a file is built for a language. + and a Python module are different kinds of thing — so a file is built for a language. A file + marked `IsHeader` is included by every translation unit that uses it, so what it defines is + written to be defined in each of them: C++ writes free functions and namespace-scope variables + `inline`, and C writes functions (lowered methods included) `static inline` and mutable variables + `static`, one copy per translation unit, since C has no inline variable + (`CFamilyGenerator.WritingHeader`). - `Coder/Ast/FunctionKind.cs`, `FunctionDefinition.cs` — what a function declares (method, constructor, destructor, operator, conversion) and where its behaviour comes from (provided, defaulted, deleted). A declaration with no statements is otherwise ambiguous between a function diff --git a/Coder.Test/Languages/HeaderLinkageTests.cs b/Coder.Test/Languages/HeaderLinkageTests.cs new file mode 100644 index 0000000..7cc626c --- /dev/null +++ b/Coder.Test/Languages/HeaderLinkageTests.cs @@ -0,0 +1,234 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Coder.Test.Languages; + +using ktsu.Coder.Ast; +using ktsu.Coder.Languages; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Tests that a generated C or C++ header can be included by more than one translation unit of a +/// program. +/// +/// +/// #pragma once keeps a header to once per translation unit and no further, so a free +/// function, a function a method is lowered to, or a mutable variable defined in it with external +/// linkage is defined once in each unit, and the program fails to link with "multiple definition" +/// (issue #423). C++ now writes them inline; C writes a function static inline and a +/// variable static, one copy per unit, since C has no inline variable. +/// +[TestClass] +public class HeaderLinkageTests +{ + private static readonly string[] CCompilers = ["cc", "gcc", "clang"]; + + private static readonly string[] CppCompilers = ["c++", "g++", "clang++"]; + + /// + /// Tests that C writes a header's functions static inline and its mutable variables + /// static. + /// + [TestMethod] + public void C_HeaderDefinitions_HaveInternalLinkage() + { + string header = new CGenerator().Generate(Header()); + + StringAssert.Contains(header, "static int limit = 5;", StringComparison.Ordinal); + StringAssert.Contains(header, "static inline int twice(int x)", StringComparison.Ordinal); + StringAssert.Contains(header, "static inline int P_get(P* self)", StringComparison.Ordinal); + StringAssert.Contains(header, "static inline int P_make(void)", StringComparison.Ordinal); + StringAssert.Contains(header, "static int scale = 8;", StringComparison.Ordinal); + StringAssert.Contains(header, "static inline int half(int x)", StringComparison.Ordinal); + } + + /// + /// Tests that C++ writes a header's free functions and namespace-scope variables inline, + /// and leaves a class's members as they were. + /// + [TestMethod] + public void Cpp_HeaderDefinitions_AreInline() + { + string header = new CppGenerator().Generate(Header()); + + StringAssert.Contains(header, "inline int limit = 5;", StringComparison.Ordinal); + StringAssert.Contains(header, "inline int twice(int x)", StringComparison.Ordinal); + StringAssert.Contains(header, "inline int scale = 8;", StringComparison.Ordinal); + StringAssert.Contains(header, "inline int half(int x)", StringComparison.Ordinal); + StringAssert.Contains(header, " int get()", StringComparison.Ordinal); + StringAssert.Contains(header, " static int make()", StringComparison.Ordinal); + } + + /// + /// Tests that a file that is not a header is written as it always was. + /// + [TestMethod] + public void ASourceFileThatIsNotAHeader_IsUnchanged() + { + SourceFile file = Header(); + file.IsHeader = false; + + string c = new CGenerator().Generate(file); + string cpp = new CppGenerator().Generate(file); + + Assert.DoesNotContain("inline", c, StringComparison.Ordinal, c); + Assert.DoesNotContain("static int limit", c, StringComparison.Ordinal, c); + Assert.DoesNotContain("static int scale", c, StringComparison.Ordinal, c); + StringAssert.Contains(c, "int twice(int x)", StringComparison.Ordinal); + StringAssert.Contains(cpp, "int limit = 5;", StringComparison.Ordinal); + Assert.DoesNotContain("inline int", cpp, StringComparison.Ordinal, cpp); + } + + /// + /// Tests that a C header included from two translation units compiles, links and runs. + /// + [TestMethod] + public void C_HeaderIncludedTwice_Links() + { + string? compiler = ToolchainHarness.FindOnPath("--version", CCompilers); + if (compiler is null) + { + Assert.Inconclusive("No C compiler on the path, so nothing was compiled."); + return; + } + + const string first = """ + #include + #include "lib.h" + + int other(void); + + int main(void) + { + P p = { 1 }; + printf("%d %d %d %d %d %d\n", limit, twice(3), P_get(&p), P_make(), half(scale), other()); + return 0; + } + + """; + + const string second = """ + #include "lib.h" + + int other(void) + { + P p = { 2 }; + return twice(limit) + P_get(&p) + P_make() + half(scale) + P_count; + } + + """; + + LinkTwoUnits(compiler, new CGenerator().Generate(Header()), "lib.h", ("a.c", first), ("b.c", second), "-std=c11 -Wall -Wextra -pedantic-errors"); + } + + /// + /// Tests that a C++ header included from two translation units compiles, links and runs. + /// + [TestMethod] + public void Cpp_HeaderIncludedTwice_Links() + { + string? compiler = ToolchainHarness.FindOnPath("--version", CppCompilers); + if (compiler is null) + { + Assert.Inconclusive("No C++ compiler on the path, so nothing was compiled."); + return; + } + + const string first = """ + #include + #include "lib.h" + + int other(); + + int main() + { + P p{}; + std::printf("%d %d %d %d %d %d\n", limit, twice(3), p.get(), P::make(), geo::half(geo::scale), other()); + return 0; + } + + """; + + const string second = """ + #include "lib.h" + + int other() + { + P p{}; + return twice(limit) + p.get() + P::make() + geo::half(geo::scale) + P::count; + } + + """; + + LinkTwoUnits(compiler, new CppGenerator().Generate(Header()), "lib.h", ("a.cpp", first), ("b.cpp", second), "-std=c++17 -Wall -Wextra -pedantic-errors"); + } + + /// + /// The issue's reproduction, widened to every kind of definition a header can hold: a mutable + /// variable and a function at file scope, a struct with an instance method, a static method and a + /// static field, and a namespace holding a variable and a function. + /// + /// The header. + private static SourceFile Header() + { + SourceFile file = new("lib") { IsHeader = true }; + file.Members.Add(new FieldDeclaration("limit", "int") { InitialValue = new LiteralExpression(5) }); + file.Members.Add(Function("twice", BinaryOperator.Multiply)); + + ClassDeclaration point = new("P") { Kind = TypeDeclarationKind.Struct }; + point.Members.Add(new FieldDeclaration("v", "int") { Visibility = Visibility.Public }); + point.Members.Add(new FieldDeclaration("count", "int") { IsStatic = true, Visibility = Visibility.Public, InitialValue = new LiteralExpression(0) }); + FunctionDeclaration get = new("get") { ReturnType = "int", Visibility = Visibility.Public }; + get.Body.Add(new ReturnStatement(new LiteralExpression(3))); + point.Members.Add(get); + FunctionDeclaration make = new("make") { ReturnType = "int", Visibility = Visibility.Public, IsStatic = true }; + make.Body.Add(new ReturnStatement(new LiteralExpression(4))); + point.Members.Add(make); + file.Members.Add(point); + + NamespaceDeclaration geo = new("geo"); + geo.Members.Add(new FieldDeclaration("scale", "int") { InitialValue = new LiteralExpression(8) }); + geo.Members.Add(Function("half", BinaryOperator.Divide)); + file.Members.Add(geo); + return file; + } + + /// + /// A function of one int that applies an operator to it and two. + /// + /// The function's name. + /// The operator. + /// The function. + private static FunctionDeclaration Function(string name, BinaryOperator op) + { + FunctionDeclaration function = new(name) { ReturnType = "int" }; + function.Parameters.Add(new Parameter("x", "int")); + function.Body.Add(new ReturnStatement(new BinaryExpression(new VariableReference("x"), op, new LiteralExpression(2)))); + return function; + } + + /// + /// Compiles two translation units that both include a header, links them, and runs the result. + /// + /// The compiler driver. + /// The generated header. + /// The name the units include it by. + /// The unit holding main. + /// The other unit. + /// The dialect and warning flags. + private static void LinkTwoUnits(string compiler, string header, string headerName, (string Name, string Source) first, (string Name, string Source) second, string flags) + { + ToolchainHarness.InTemporaryDirectory(directory => + { + File.WriteAllText(Path.Combine(directory, headerName), header); + File.WriteAllText(Path.Combine(directory, first.Name), first.Source); + File.WriteAllText(Path.Combine(directory, second.Name), second.Source); + + (int exitCode, string output) = ToolchainHarness.Run(compiler, $"{flags} {first.Name} {second.Name} -o linked", directory); + Assert.AreEqual(0, exitCode, $"{compiler} could not build the two units:{Environment.NewLine}{header}{Environment.NewLine}{output}"); + + (int ran, string printed) = ToolchainHarness.Run(Path.Combine(directory, "linked"), string.Empty, directory); + Assert.AreEqual(0, ran, printed); + Assert.AreEqual("5 6 3 4 4 21", printed.Trim(), header); + }); + } +} diff --git a/Coder/Languages/CFamilyGenerator.cs b/Coder/Languages/CFamilyGenerator.cs index 3f506b3..5fdf798 100644 --- a/Coder/Languages/CFamilyGenerator.cs +++ b/Coder/Languages/CFamilyGenerator.cs @@ -66,6 +66,18 @@ protected sealed override void GenerateFunctionDeclaration(FunctionDeclaration f /// protected abstract string SpellType(TypeReference type); + /// + /// Gets a value indicating whether the file being generated is a header. + /// + /// + /// A header is included by every translation unit that uses it, and #pragma once keeps it + /// to once per unit and no further: a definition with external linkage in it is defined once in + /// each unit and the program does not link. So what a header defines is written so that a + /// definition in every unit is allowed, as inline in C++ and as static or + /// static inline in C, which has no inline variable (#423). + /// + protected bool WritingHeader => GenerationRoot is SourceFile { IsHeader: true }; + /// /// Gets the file extension (without the dot) used for a header in this language. /// diff --git a/Coder/Languages/CGenerator.cs b/Coder/Languages/CGenerator.cs index 5aacf7a..bcf7480 100644 --- a/Coder/Languages/CGenerator.cs +++ b/Coder/Languages/CGenerator.cs @@ -254,9 +254,20 @@ protected override void GenerateFunction(FunctionDeclaration funcDecl, CodeBlock // C has no generics, so what a function is written over is written down, as for a type. WriteTypeParametersDown(funcDecl.TypeParameters, code); - // Internal linkage is the only privacy C has, and it is what a private function wants: the - // declaration is visible to this translation unit and to nothing else. - if (funcDecl.Visibility == Visibility.Private) + // A declaration with no definition is a prototype, which is all C has to offer for either of + // them: an abstract declaration is one an implementation must supply, and a defaulted one is + // one the language would have supplied had it been C++. + bool isPrototype = funcDecl.IsAbstract || funcDecl.Definition == FunctionDefinition.Defaulted; + + // A definition in a header is made in every translation unit that includes it, and only + // `static inline` lets each of them have one without the program defining it twice (#423). + // Otherwise internal linkage is the only privacy C has, and it is what a private function + // wants: the declaration is visible to this translation unit and to nothing else. + if (WritingHeader && !isPrototype) + { + code.Write("static inline "); + } + else if (funcDecl.Visibility == Visibility.Private) { code.Write("static "); } @@ -267,10 +278,7 @@ protected override void GenerateFunction(FunctionDeclaration funcDecl, CodeBlock code.Write(SpellFunctionName(funcDecl, enclosingType)); WriteParameterList(ParametersOf(funcDecl, enclosingType), code); - // A declaration with no definition is a prototype, which is all C has to offer for either of - // them: an abstract declaration is one an implementation must supply, and a defaulted one is - // one the language would have supplied had it been C++. - if (funcDecl.IsAbstract || funcDecl.Definition == FunctionDefinition.Defaulted) + if (isPrototype) { code.WriteLine(";"); return; @@ -1150,7 +1158,10 @@ private void WriteFileScopeField(FieldDeclaration field, string name, CodeBlocke TypeReference type = field.Type ?? new TypeReference(UnknownTypeName); - if (field.IsConstant || field.IsStatic || field.Visibility == Visibility.Private) + // C has no inline variable, so a mutable one in a header is static too: every translation + // unit that includes the header has its own copy, which is the one way the header can define + // it without the program defining it twice (#423). + if (field.IsConstant || field.IsStatic || field.Visibility == Visibility.Private || WritingHeader) { code.Write("static "); } diff --git a/Coder/Languages/CppGenerator.cs b/Coder/Languages/CppGenerator.cs index cd68dd5..9f2ae31 100644 --- a/Coder/Languages/CppGenerator.cs +++ b/Coder/Languages/CppGenerator.cs @@ -298,6 +298,11 @@ protected override void GenerateFunction(FunctionDeclaration funcDecl, CodeBlock code.Write("static "); } + if (NeedsInline(funcDecl, enclosingType)) + { + code.Write("inline "); + } + // An abstract declaration is virtual whether or not it was asked to be: there is nothing else // `= 0` could mean. if (funcDecl.IsVirtual || funcDecl.IsAbstract) @@ -366,6 +371,24 @@ protected override void GenerateFunction(FunctionDeclaration funcDecl, CodeBlock } } + /// + /// Reports whether a function has to say inline to be defined in a header. + /// + /// The declaration being emitted. + /// The type it is a member of, or null for a free function. + /// True for a free function defined in a header that is not inline already. + /// + /// A function defined in a class is inline already, and so are a friend defined there and a + /// constexpr one; anything else defined in a header is defined again in every translation + /// unit that includes it, and the program does not link (#423). + /// + private bool NeedsInline(FunctionDeclaration funcDecl, string? enclosingType) => + WritingHeader + && enclosingType is null + && !funcDecl.IsFriend + && !funcDecl.IsCompileTimeEvaluable + && !funcDecl.IsAbstract; + /// /// Writes what the type's members start at, between a constructor's signature and its body. /// @@ -1038,7 +1061,10 @@ private string SpellStorage(FieldDeclaration field) return insideType > 0 ? "inline static " : "static "; } - return string.Empty; + // A mutable variable at namespace scope has external linkage, so a header defining one + // defines it again in every translation unit that includes it. C++17's inline variable is + // one object however many units define it (#423). + return insideType == 0 && WritingHeader ? "inline " : string.Empty; } ///