Name mangling for imported internal declarations (before symbol resolution)#360
Name mangling for imported internal declarations (before symbol resolution)#360bamarsha wants to merge 23 commits intofeatures/access-modifiersfrom
Conversation
Some tests failing due to bugs.
|
Quick question, but how does this interact if at all with the emulation feature? Can internal operations be given native implementations with C# code, or will that break upon renaming? |
|
@cgranade My first thought is that yes, if an internal Q# declaration relies on a C# implementation with the same name to work, it will probably break when used as a package reference. I'm not familiar with the emulation feature, however, is there somewhere I can learn more about this? |
Thanks for the help! The example I was thinking of was the emulation of the |
My take is that supporting the emulation feature only for public callables seems reasonable. Anything internal means it's subject to change and external packages should not depend on it (we could choose to ship emulation capabilites with the library itself in that case, but I see no immediate need anywhere). |
|
IMO the same reasons to make a non-emulated operation internal could apply to an emulated operation as well. For example, if you want to have an emulated operation with several parameters that are used within the project, but only want to expose a subset of those parameters publicly. It would be nice if it was supported, which I think could work if we avoided name mangling in favor of updating the symbol table to support multiple declarations of internal things. If we don't support it, there should be an error message at compile time. Maybe by making internal operations not partial classes in the generated C# code. |
|
@cgranade @bettinaheim Actually, I'm not sure that native operations will be broken by name mangling. Name mangling doesn't affect the C# classes that are loaded as part of a package reference, so the C# versions of the operations calling the internal operation will still use the non-mangled name. It's only in the Q# syntax tree that it's renamed, and I thought that C# code is only generated for Q# code defined in the current project, not in referenced assemblies? |
Making internal operations not partial classes in the generated C# code seems like a good idea. You are right about reasons for why in general one might want to support emulation for internal things, but given that we currently don't have a concrete use case, I wouldn't invest into that scenario for now. |
That's correct. You mean regarding supporting emulation for those? You are right, that should all work (unless/until we drop the C# generation for libraries). |
…marshallsa/rename-references
bamarsha
changed the title
|
Replaced by #370. |
…#345) * Keep track of modifiers for callables and types * Make Modifiers a struct so serialization works * Remove runtime tests until compiler version is updated * Add converter and update serialization test * Update transformation and walker * Move modifiers out of the signatures * Fix attribute reader test * Undo superfluous changes * Update documentation for DefaultAccess * Re-enable execution tests * Add parsing for access modifiers * Update syntax highlighting for private and internal * Add TODOs for parsing the final syntax * Fix code style for creating modifiers record * Parse modifiers before fragment headers * Include modifiers in the fragment header parser * Move private/internal to a separate list * Make getFragments a normal value * Remove let rec from fragment parsers * Move fragment header consistency check to do-block * Add access modifiers to code completion * Include modifiers in code completion tests * optR makes more sense this way * Fix fragment being created without consuming input * Check for access when resolving callables * Check access when resolving types * Add error code for inaccessible type in a qualified symbol * Show specific error message for inaccessible type in open NS * Add custom error message for inaccessible callable (unqualified) - needs cleanup * Show specific error message for inaccessible qualified callables * Rename ContainsCallable/Type to TryFindCallable/Type * Make errors for using less accessible types in more accessible declarations * Reword description of Ambiguous * Update comments for NamespacesContaining{Callable, Type} * Clarify checkUdt parameter in doc comment * Update src/QsCompiler/Core/SymbolTable.fs Co-Authored-By: bettinaheim <34236215+bettinaheim@users.noreply.github.com> * NonSupportedException -> NotSupportedException * Add tests for access modifiers * Add a few more tests * Add tests for access modifiers in a referenced assembly * Update access modifiers test namespace * Skip access modifier tests for now * Add tests for type constructor accessibility * Re-enable tests except for redeclarations of references * Replace tabs with spaces * Use more descriptive names * Add access modifiers to hover information * Add access modifiers to Q# code output * Add test for skipping inaccessible items in documentation * Skip private and internal items when generating documentation * Add copyright headers * Add doc comment for new test * Clarify doc comment for WriteToStream * Hide inaccessible callables and types from code completion * Update namespace suggestion code actions for access modifiers * Add QsNullable.Bind * Refactor this if statement a little * Move nsName == null check earlier * Update doc comments for namespace suggestions * Update symbol table documentation for access modifiers * Remove the private access modifier * Move internal keyword to operations syntax group * Move "References" tests to other test groups * Allow shadowing imported internal callables * Mangle names in CompilationUnit.Build * Add TODO about reversing name mangling * Update comments about accessibility in Namespace class * Check partial NS before ref in TryAddCallableSpecialization * Skip calling FromSingleSource in NS.TryFindCallable * Allow shadowing imported internal types * Enable redeclaration test * Remove unused functions * Check source then references in TryGetAttributeDeclaredIn * Fix bug in PossibleQualifications * Fix bug in resolveInOpenNamespaces * Re-enable exception in SpecializationsDefinedInAllSources * Re-enable assertion about external specializations * Add name mangling tests * Add test for using same internal name in multiple references * No diagnostics for "conflicting" internal names * Move ResolutionResult to SymbolResolution.fs * Add comment about name tagging * Use Library2 instead of Library1Copy * Ignore async tasks instead of waiting * Disable multiple reference test * Use PascalCase in ResolutionResult module * Check if source is in references dictionary * Use reversible name mangling * Add missing doc comments * Add missing underscores * Remove unnecessary thunking * Remove unnecessary nullability * Add test for grouping internal specializations * Refactor creating references * Skip test for now * Pulling in changes from PR 358 (#375) * Bringing over change from PR #360; avoid shadowing * Re-enable multiple internal references test * Update symbol table * Group specializations by source file * Fix alignment * Only group specializations by source if callable is internal * Re-add explicit this * getting updates from master * Undo checks for current NS in code actions/completion * Check if methodDecl is null Co-authored-by: bettinaheim <34236215+bettinaheim@users.noreply.github.com> Co-authored-by: Bettina Heim <beheim@microsoft.com>
4 participants
Use name mangling so that internal declarations in references can't conflict with each other or with the source files in the compilation unit.
Part of #259.