-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[doc] Add documentation for clang-change-namespace #148277
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,155 @@ | ||
====================== | ||
Clang-Change-Namespace | ||
====================== | ||
|
||
.. contents:: | ||
|
||
.. toctree:: | ||
:maxdepth: 1 | ||
|
||
:program:`clang-change-namespace` can be used to change the surrounding | ||
namespaces of class/function definitions. | ||
|
||
Classes/functions in the moved namespace will have new namespaces while | ||
references to symbols (e.g. types, functions) which are not defined in the | ||
changed namespace will be correctly qualified by prepending namespace specifiers | ||
before them. This will try to add shortest namespace specifiers possible. | ||
|
||
When a symbol reference needs to be fully-qualified, this adds a `::` prefix to | ||
the namespace specifiers unless the new namespace is the global namespace. For | ||
Comment on lines
+18
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not certain I understand this. Is this saying:
with
will produce
will produce There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understood this as fully-qualifying the namespace when necessary to refer to the correct namespace. For example, namespace other::foo {
a::A thing;
} // namespace other::foo
namespace foo {
a::A thing;
} // namespace foo
namespace other {
a::A thing;
struct foo {
a::A thing;
};
a::A thing2;
} // namespace other becomes namespace other::foo {
::foo::A thing;
} // namespace other::foo
namespace foo {
A thing;
} // namespace foo
namespace other {
foo::A thing;
struct foo {
::foo::A thing;
};
::foo:A thing2;
} // namespace other @kwk is that understanding correct? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried running the binary and after looking at the tests, I believe I've misunderstood. It looks like the tool only changes symbol references within the moved namespace. It does not update any symbol references from outside the moved namespace that point to symbols inside the moved namespace (which would no longer reference a valid symbol after the changes are applied). Concrete example: // a.cc
namespace old {
struct foo {};
} // namespace old
namespace b {
old::foo g_foo;
} // namespace b
I think that is worth clarifying that this only updates symbol references in the moved namespace. (Unfortunately, I feel like that makes this tool much less useful.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps not a question on the documentation, but rather the tool. Why is the global namespace special in this regard? It seems like there could be name ambiguities that arise with moving to the global namespace.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tJener that is correct. As @nikic pointed out in a chat, this tool looks unmaintained. Most of the content changes in here look like as if they are 9 years old. The fact that you, @tJener and @AaronBallman ask these very specific questions lets me think that it is also not in active use. I'm going to go on PTO soon but I will put all of your questions on my list to address when I'm back. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I suspect you may be looking for an Eric Liu who appears to have written the majority of the tool, which is not me. Which is to say that my lack of familiarity with this tool shouldn't really mean much. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Consider this example to showcase ambiguities in names being used more than once: cat test.cc
namespace a {
class A {
int classAFromWithinNamespace_a;
};
} // namespace a
class A {
int classAInGlobalNamespace;
}; Then run class A {
int classAFromWithinNamespace_a;
};
class A {
int classAInGlobalNamespace;
}; The re-factoring looks correct but the code will not compile due to the name duplication. I'd say it is not up to the tool to ensure compilability in that sense. Of course, if you agree to this, then this MUST be documented. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah, the code is basically unmaintained at this point, though I suspect that's because it's in "good enough" state that it doesn't require much active work. I'd say let's document the state of things as they are. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think that's worth documenting. |
||
classes, only classes that are declared/defined in the given namespace in | ||
specified files will be moved: forward declarations will remain in the old | ||
namespace. The will be demonstrated in the next example. | ||
|
||
Example usage | ||
------------- | ||
|
||
For example, consider this `test.cc` example here with the forward declared | ||
class `FWD` and the defined class `A`, both in the namespace `a`. | ||
|
||
.. code-block:: c++ | ||
|
||
namespace a { | ||
class FWD; | ||
class A { | ||
FWD *fwd; | ||
}; | ||
} // namespace a | ||
|
||
And now let's change the namespace `a` to `x`. | ||
|
||
.. code-block:: console | ||
|
||
clang-change-namespace \ | ||
--old_namespace "a" \ | ||
--new_namespace "x" \ | ||
--file_pattern "test.cc" \ | ||
--i \ | ||
test.cc | ||
|
||
Note that in the code below there's still the forward decalred class `FWD` that | ||
stayed in the namespace `a`. It wasn't moved to the new namespace because it | ||
wasn't defined/declared here in `a` but only forward declared. | ||
|
||
.. code-block:: c++ | ||
|
||
namespace a { | ||
class FWD; | ||
} // namespace a | ||
namespace x { | ||
|
||
class A { | ||
a::FWD *fwd; | ||
}; | ||
} // namespace x | ||
|
||
|
||
Another example | ||
--------------- | ||
|
||
Consider this `test.cc` file: | ||
|
||
.. code-block:: c++ | ||
|
||
namespace na { | ||
class X {}; | ||
namespace nb { | ||
class Y { | ||
X x; | ||
}; | ||
} // namespace nb | ||
} // namespace na | ||
|
||
To move the definition of class `Y` from namespace `na::nb` to `x::y`, run: | ||
|
||
.. code-block:: console | ||
|
||
clang-change-namespace \ | ||
--old_namespace "na::nb" \ | ||
--new_namespace "x::y" \ | ||
--file_pattern "test.cc" \ | ||
--i \ | ||
test.cc | ||
|
||
This will overwrite `test.cc` to look like this: | ||
|
||
.. code-block:: c++ | ||
|
||
namespace na { | ||
class X {}; | ||
|
||
} // namespace na | ||
namespace x { | ||
namespace y { | ||
class Y { | ||
na::X x; | ||
}; | ||
} // namespace y | ||
} // namespace x | ||
|
||
Note, that we've successfully moved the class `Y` from namespace `na::nb` to | ||
namespace `x::y`. | ||
|
||
:program:`clang-change-namespace` Command Line Options | ||
====================================================== | ||
|
||
.. option:: --allowed_file=<string> | ||
|
||
A file containing regexes of symbol names that are not expected to be updated | ||
when changing namespaces around them. | ||
|
||
.. option:: --dump_result | ||
|
||
Dump new file contents in YAML, if specified. | ||
|
||
.. option:: --extra-arg=<string> | ||
|
||
Additional argument to append to the compiler command line | ||
|
||
.. option:: --extra-arg-before=<string> | ||
|
||
Additional argument to prepend to the compiler command line | ||
|
||
.. option:: --file_pattern=<string> | ||
|
||
Only rename namespaces in files that match the given pattern. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it'd be worth specifying that this is a regular expression, as opposed to some other syntax (e.g. glob). |
||
|
||
.. option:: -i | ||
|
||
Inplace edit <file>s, if specified. | ||
|
||
.. option:: --new_namespace=<string> | ||
|
||
New namespace. | ||
|
||
.. option:: --old_namespace=<string> | ||
Comment on lines
+141
to
+145
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the global namespace represented by an empty string? Are there other things supported like making a namespace inline by using |
||
|
||
Old namespace. | ||
|
||
.. option:: -p <string> | ||
|
||
Build path | ||
|
||
.. option:: --style=<string> | ||
|
||
The style name used for reformatting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AaronBallman I'm open for this but I went with the way it's done by
clang-include-fixer
andclang-tidy
. Changing it would break with the convention over there.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving it as-is works for me then, thank you!