-
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?
Conversation
This adds rst documentation for the `clang-change-namespace` program. I ran the examples locally to ensure they work. See llvm#35519
@llvm/pr-subscribers-clang-tools-extra Author: Konrad Kleine (kwk) ChangesThis adds rst documentation for the I ran the examples locally to ensure they work. See #35519 Full diff: https://github.com/llvm/llvm-project/pull/148277.diff 2 Files Affected:
diff --git a/clang-tools-extra/docs/clang-change-namespace.rst b/clang-tools-extra/docs/clang-change-namespace.rst
new file mode 100644
index 0000000000000..23c3dcc8b7412
--- /dev/null
+++ b/clang-tools-extra/docs/clang-change-namespace.rst
@@ -0,0 +1,158 @@
+======================
+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
+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++
+ :linenos:
+
+ 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++
+ :linenos:
+
+ 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++
+ :linenos:
+
+ 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++
+ :linenos:
+
+ 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.
+
+.. option:: -i
+
+ Inplace edit <file>s, if specified.
+
+.. option:: --new_namespace=<string>
+
+ New namespace.
+
+.. option:: --old_namespace=<string>
+
+ Old namespace.
+
+.. option:: -p <string>
+
+ Build path
+
+.. option:: --style=<string>
+
+ The style name used for reformatting.
diff --git a/clang-tools-extra/docs/index.rst b/clang-tools-extra/docs/index.rst
index 9f7324fcf7419..3f3a99d1b70c6 100644
--- a/clang-tools-extra/docs/index.rst
+++ b/clang-tools-extra/docs/index.rst
@@ -17,6 +17,7 @@ Contents
clang-tidy/index
clang-include-fixer
+ clang-change-namespace
modularize
pp-trace
clangd <https://clangd.llvm.org/>
|
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.
Thank you for working on the documentation for this, it's really appreciated! I had a few questions, but in general this is looking good.
@@ -0,0 +1,155 @@ | |||
====================== | |||
Clang-Change-Namespace |
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.
Clang-Change-Namespace | |
clang-change-namespace |
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
and clang-tidy
. Changing it would break with the convention over there.
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 |
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.
I'm not certain I understand this. Is this saying:
namespace a {
class A {
};
} // namespace a
a::A thing;
with
clang-change-namespace \
--old_namespace "a" \
--new_namespace "" \
--file_pattern "test.cc" \
--i \
test.cc
will produce A thing;
but
clang-change-namespace \
--old_namespace "a" \
--new_namespace "foo" \
--file_pattern "test.cc" \
--i \
test.cc
will produce ::foo::A thing;
?
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.
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?
.. option:: --new_namespace=<string> | ||
|
||
New namespace. | ||
|
||
.. option:: --old_namespace=<string> |
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.
Is the global namespace represented by an empty string? Are there other things supported like making a namespace inline by using "inline <name>"
?
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 |
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.
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.
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.
@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 comment
The 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.
This adds rst documentation for the
clang-change-namespace
program.I ran the examples locally to ensure they work.
Fixes #35519