Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Brief documentation for the current name mangling scheme #4286

Merged
merged 15 commits into from
Sep 14, 2024
Merged

Brief documentation for the current name mangling scheme #4286

merged 15 commits into from
Sep 14, 2024

Conversation

dwblaikie
Copy link
Contributor

Documents the scheme thus far, as implemented in a548eff

@github-actions github-actions bot requested a review from geoffromer September 9, 2024 17:56
geoffromer
geoffromer reviewed Sep 9, 2024
View reviewed changes
jonmeow
jonmeow reviewed Sep 9, 2024
View reviewed changes
Copy link
Contributor

@jonmeow jonmeow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought I might add on here... I think geoffromer might be missing a couple things currently supported. It's also good to keep in mind the style guide as we write documentation.

dwblaikie and others added 4 commits September 9, 2024 14:44
@dwblaikie @geoffromer
Remove incorrect empty lines
f4dda62 
Co-authored-by: Geoff Romer <gromer@google.com>
@dwblaikie @CarbonInfraBot
Line wrapping
fbc3dac 
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
@dwblaikie
Add examples, number lists, include main mangling
7ef62fc
@dwblaikie
Add missing namespace from example
4ac1905
jonmeow
jonmeow approved these changes Sep 13, 2024
View reviewed changes
Copy link
Contributor

@jonmeow jonmeow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks good, and I think everything is covered now. Just some final comments, but from my perspective you're free to merge after they're addressed.

geoffromer
geoffromer approved these changes Sep 13, 2024
View reviewed changes
Copy link
Contributor

@geoffromer geoffromer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor nit, then this looks good to me once @jonmeow's comments are addressed.

dwblaikie and others added 8 commits September 13, 2024 11:34
@dwblaikie @geoffromer
More clearly introduce the scope mangling scheme
34b2b4d 
Co-authored-by: Geoff Romer <gromer@google.com>
@dwblaikie @jonmeow
correct mangled name
b0ba8a6 
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
@dwblaikie @jonmeow
Remove perfunctory period
e7d0d2a 
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
@dwblaikie @jonmeow
Add period
48b1f9f 
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
@dwblaikie @jonmeow
Capitalize start of sentence
633006d 
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
@dwblaikie @jonmeow
Add comma
a1cd41d 
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
@dwblaikie @jonmeow
Avoid latin abbreviations
bbd55f0 
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
@dwblaikie
Merge branch 'trunk' into mangling_documentation
cae05dc
@dwblaikie dwblaikie enabled auto-merge September 13, 2024 21:16
dwblaikie and others added 2 commits September 13, 2024 21:26
@dwblaikie
Merge branch 'trunk' into mangling_documentation
db5d32e
@dwblaikie @CarbonInfraBot
Line wrapping
52bc373 
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
@dwblaikie dwblaikie added this pull request to the merge queue Sep 14, 2024
@dwblaikie dwblaikie removed this pull request from the merge queue due to a manual request Sep 14, 2024
@dwblaikie dwblaikie added this pull request to the merge queue Sep 14, 2024
Merged via the queue into carbon-language:trunk with commit 580e845 Sep 14, 2024
8 checks passed
@dwblaikie dwblaikie deleted the mangling_documentation branch September 14, 2024 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CarbonInfraBot CarbonInfraBot CarbonInfraBot left review comments

@geoffromer geoffromer geoffromer approved these changes

@jonmeow jonmeow jonmeow approved these changes

Assignees
No one assigned
Labels
toolchain
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dwblaikie @geoffromer @jonmeow @CarbonInfraBot