Skip to content

Commit 1626f87

Browse files
dalexsotoCopilot
andauthored
[sharpie] Fix --scope path matching (#24882)
When using `sharpie bind --header <file> --scope <dir>`, the --scope argument was stored verbatim without path normalization. Since Clang always reports declaration source locations as absolute paths, a relative --scope value (e.g. `MyFramework.framework/Headers`) would never match the absolute filename from `presumedLoc.FileName`, causing IsInScope() to filter out every declaration and produce zero output files — even though parsing succeeded. Additionally, the StartsWith comparison in IsInScope() could produce false positive matches when one directory name was a prefix of another (e.g. scope `/tmp/scope` would incorrectly match files in `/tmp/scopeextra/`). ~~Finally, when binding a very large number of Objective-C headers (200+), ClangSharp can throw an ArgumentOutOfRangeException with parameter name "handle" during AST traversal, due to cursor/type handle misclassification in its managed wrapper layer. Sharpie caught this exception but reported the raw, opaque message ("Specified argument was out of the range of valid values"), giving users no guidance on how to work around the issue.~~ -> Moved to dotnet/ClangSharp#690 Changes: 1. Tools.cs: Normalize --scope paths to absolute via Path.GetFullPath() when parsing CLI arguments, matching the behavior already used by --framework mode (which calls Path.GetFullPath on SourceFramework in ResolveFramework()). 2. ObjectiveCBinder.cs (IsInScope): Append a trailing directory separator to scope directory paths before the StartsWith check, so that `/tmp/scope/` does not falsely match `/tmp/scopeextra/`. ~~3. BindingResult.cs (ReportUnexpectedError): Detect the specific ArgumentOutOfRangeException("handle") pattern from ClangSharp and report an actionable error message that explains the root cause (translation unit too complex) and suggests workarounds (bind fewer headers, use --scope).~~ 4. Tests: Added 5 new test cases: - Scope_RelativePath: verifies relative --scope paths produce correct output after normalization - Scope_FiltersOutOfScopeDeclarations: verifies that only declarations from in-scope headers are bound - Scope_PrefixDoesNotFalseMatch: verifies that scope "/foo/bar" does not match "/foo/barbaz/header.h" - ~~HandleCrash_ReportsUsefulError: verifies the improved error message for ClangSharp handle exceptions~~ - ~~HandleCrash_OtherExceptionsUnchanged: verifies that other exception types still report their original message~~ --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 15251bc commit 1626f87

3 files changed

Lines changed: 178 additions & 2 deletions

File tree

tests/sharpie/Sharpie.Bind.Tests/ObjectiveCClass.cs

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,4 +456,175 @@ public struct MyStruct {
456456
Assert.That (bindings.AdditionalFiles ["ApiDefinition.cs"].Trim (), Is.EqualTo (expectedApiDefinitionBindings.Trim ()), "Api definition");
457457
Assert.That (bindings.AdditionalFiles ["StructsAndEnums.cs"].Trim (), Is.EqualTo (expectedStructAndEnumsBindings.Trim ()), "Struct and enums");
458458
}
459+
460+
[Test]
461+
public void Scope_RelativePath ()
462+
{
463+
// Verify that --scope with a relative path works correctly
464+
// (the relative path should be resolved to absolute before matching).
465+
var binder = new BindTool ();
466+
var tmpdir = Cache.CreateTemporaryDirectory ();
467+
var subdir = Path.Combine (tmpdir, "headers");
468+
Directory.CreateDirectory (subdir);
469+
470+
// Create a header in the scoped directory
471+
var scopedHeader = Path.Combine (subdir, "InScope.h");
472+
File.WriteAllText (scopedHeader,
473+
"""
474+
@interface InScopeClass {
475+
}
476+
@property int Value;
477+
@end
478+
""");
479+
480+
// Create an umbrella header that includes both
481+
var mainHeader = Path.Combine (tmpdir, "main.h");
482+
File.WriteAllText (mainHeader, $"#import \"{scopedHeader}\"\n");
483+
484+
binder.SplitDocuments = false;
485+
binder.SourceFile = mainHeader;
486+
binder.OutputDirectory = tmpdir;
487+
488+
// Use a relative scope path (the bug was that this produced empty output)
489+
var oldCwd = Environment.CurrentDirectory;
490+
try {
491+
Environment.CurrentDirectory = tmpdir;
492+
binder.DirectoriesInScope.Add (Path.GetFullPath ("headers"));
493+
} finally {
494+
Environment.CurrentDirectory = oldCwd;
495+
}
496+
497+
Configuration.IgnoreIfIgnoredPlatform (binder.Platform);
498+
binder.PlatformAssembly = Extensions.GetPlatformAssemblyPath (binder.Platform);
499+
binder.ClangResourceDirectory = Extensions.GetClangResourceDirectory ();
500+
var bindings = binder.BindInOrOut ();
501+
var expectedBindings =
502+
"""
503+
using Foundation;
504+
505+
// @interface InScopeClass
506+
interface InScopeClass {
507+
// @property int Value;
508+
[Export ("Value")]
509+
int Value { get; set; }
510+
}
511+
512+
""";
513+
bindings.AssertSuccess (expectedBindings);
514+
}
515+
516+
[Test]
517+
public void Scope_FiltersOutOfScopeDeclarations ()
518+
{
519+
// Verify that declarations from headers outside the scope directory are not bound.
520+
var binder = new BindTool ();
521+
var tmpdir = Cache.CreateTemporaryDirectory ();
522+
var scopedDir = Path.Combine (tmpdir, "scoped");
523+
var unscopedDir = Path.Combine (tmpdir, "unscoped");
524+
Directory.CreateDirectory (scopedDir);
525+
Directory.CreateDirectory (unscopedDir);
526+
527+
var scopedHeader = Path.Combine (scopedDir, "Scoped.h");
528+
File.WriteAllText (scopedHeader,
529+
"""
530+
@interface ScopedClass {
531+
}
532+
@property int A;
533+
@end
534+
""");
535+
536+
var unscopedHeader = Path.Combine (unscopedDir, "Unscoped.h");
537+
File.WriteAllText (unscopedHeader,
538+
"""
539+
@interface UnscopedClass {
540+
}
541+
@property int B;
542+
@end
543+
""");
544+
545+
var mainHeader = Path.Combine (tmpdir, "main.h");
546+
File.WriteAllText (mainHeader, $"#import \"{scopedHeader}\"\n#import \"{unscopedHeader}\"\n");
547+
548+
binder.SplitDocuments = false;
549+
binder.SourceFile = mainHeader;
550+
binder.OutputDirectory = tmpdir;
551+
binder.DirectoriesInScope.Add (scopedDir);
552+
Configuration.IgnoreIfIgnoredPlatform (binder.Platform);
553+
binder.PlatformAssembly = Extensions.GetPlatformAssemblyPath (binder.Platform);
554+
binder.ClangResourceDirectory = Extensions.GetClangResourceDirectory ();
555+
var bindings = binder.BindInOrOut ();
556+
557+
// Only ScopedClass should be in the output, not UnscopedClass
558+
var expectedBindings =
559+
"""
560+
using Foundation;
561+
562+
// @interface ScopedClass
563+
interface ScopedClass {
564+
// @property int A;
565+
[Export ("A")]
566+
int A { get; set; }
567+
}
568+
569+
""";
570+
bindings.AssertSuccess (expectedBindings);
571+
}
572+
573+
[Test]
574+
public void Scope_PrefixDoesNotFalseMatch ()
575+
{
576+
// Verify that a scope of "/foo/bar" does not match "/foo/barbaz/header.h"
577+
// (the scope must be a proper directory prefix with separator).
578+
var binder = new BindTool ();
579+
var tmpdir = Cache.CreateTemporaryDirectory ();
580+
var scopedDir = Path.Combine (tmpdir, "scope");
581+
var falseMatchDir = Path.Combine (tmpdir, "scopeextra");
582+
Directory.CreateDirectory (scopedDir);
583+
Directory.CreateDirectory (falseMatchDir);
584+
585+
var scopedHeader = Path.Combine (scopedDir, "Good.h");
586+
File.WriteAllText (scopedHeader,
587+
"""
588+
@interface GoodClass {
589+
}
590+
@property int X;
591+
@end
592+
""");
593+
594+
var falseMatchHeader = Path.Combine (falseMatchDir, "Bad.h");
595+
File.WriteAllText (falseMatchHeader,
596+
"""
597+
@interface BadClass {
598+
}
599+
@property int Y;
600+
@end
601+
""");
602+
603+
var mainHeader = Path.Combine (tmpdir, "main.h");
604+
File.WriteAllText (mainHeader, $"#import \"{scopedHeader}\"\n#import \"{falseMatchHeader}\"\n");
605+
606+
binder.SplitDocuments = false;
607+
binder.SourceFile = mainHeader;
608+
binder.OutputDirectory = tmpdir;
609+
binder.DirectoriesInScope.Add (scopedDir);
610+
Configuration.IgnoreIfIgnoredPlatform (binder.Platform);
611+
binder.PlatformAssembly = Extensions.GetPlatformAssemblyPath (binder.Platform);
612+
binder.ClangResourceDirectory = Extensions.GetClangResourceDirectory ();
613+
var bindings = binder.BindInOrOut ();
614+
615+
// Only GoodClass should appear (from "scope/"), not BadClass (from "scopeextra/")
616+
var expectedBindings =
617+
"""
618+
using Foundation;
619+
620+
// @interface GoodClass
621+
interface GoodClass {
622+
// @property int X;
623+
[Export ("X")]
624+
int X { get; set; }
625+
}
626+
627+
""";
628+
bindings.AssertSuccess (expectedBindings);
629+
}
459630
}

tools/sharpie/Sharpie.Bind/ObjectiveCBinder.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,12 @@ bool IsInScope (Decl? decl)
154154
if (string.IsNullOrEmpty (fn))
155155
return true;
156156
foreach (var dir in DirectoriesInScope) {
157-
if (fn.StartsWith (dir, StringComparison.Ordinal))
157+
// Ensure the scope directory ends with a directory separator so that
158+
// a scope of "/foo/bar" doesn't falsely match "/foo/barbaz/header.h".
159+
var normalizedDir = dir.EndsWith (Path.DirectorySeparatorChar) || dir.EndsWith (Path.AltDirectorySeparatorChar)
160+
? dir
161+
: dir + Path.DirectorySeparatorChar;
162+
if (fn.StartsWith (normalizedDir, StringComparison.Ordinal))
158163
return true;
159164
}
160165

tools/sharpie/Sharpie.Bind/Tools.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public static int Bind (string [] arguments)
3333
{ "s|sdk=", "Target SDK.", v => binder.Sdk = v },
3434
{ "f|framework=", "The input framework to bind. Implies setting the scope (--scope) to the framework, setting the namespace (--namespace) to the name of the framework, and no other sources/headers can be specified. If the framework provides an 'Info.plist' with SDK information (DTSDKName), the '-sdk' option will be implied as well (if not manually specified).", v => binder.SourceFramework = v },
3535
{ "header=", "The input header file to bind. This can also be a .framework directory.", v => binder.SourceFile = v },
36-
{ "scope=", "Restrict following #include and #import directives declared in header files to within the specified DIR directory.", v => binder.DirectoriesInScope.Add (v) },
36+
{ "scope=", "Restrict following #include and #import directives declared in header files to within the specified DIR directory.", v => binder.DirectoriesInScope.Add (Path.GetFullPath (v)) },
3737
{ "c|clang", "All arguments after this argument are not processed by Objective Sharpie and are proxied directly to Clang.", v => { } },
3838
{ "clang-resource-dir=", "Specify the Clang resource directory.", v => binder.ClangResourceDirectory = v },
3939
{ "platform-assembly=", "Specify the platform assembly to use for binding.", v => binder.PlatformAssembly = v },

0 commit comments

Comments
 (0)