Use C++17 nested declarations and add CppParser support, closes #5234 by mikomikotaishi · Pull Request #5235 · pocoproject/poco

mikomikotaishi · 2026-03-09T12:58:05Z

This pull request closes #5234, and replaces the repeated namespace declarations with the new C++17 nested namespace declarations for a concise declaration of multiple nested namespaces. (The library is C++20, and thus should not be broken by this change.)

// currently...
namespace Poco {
namespace Net {
namespace ODBC { 
    // ...
}}} // namespace Poco::Data::ODBC

// proposed new-style (C++17 onward)
namespace Poco::Net::ODBC {
    // ...
} // namespace Poco::Data::ODBC

This was done using a Python script (and a small part by hand for more complex files), and the CI tests don't report any build errors so I assume it was done correctly, but as always if something is incorrect please bring it to my attention.

As CppParser requires nested namespace support as well to generate the new docs, I have updated it to support nested namespaces as well as a small test case to the testing suite.

mikomikotaishi · 2026-03-09T15:45:01Z

This is a change that doesn't actually change the usage of the library at the consumer end, so could we merge this one before 1.16.0? I think it falls under code tidiness and would probably be better suited to be merged sooner rather than later.

mikomikotaishi · 2026-03-09T16:06:38Z

The CodeQL errors seem to not be caused by anything made by this PR, as the affected errors are not mentioning changes made in this PR, so I won't address those in this PR.

obiltschnig · 2026-03-11T14:19:31Z

This change will also require someone to add support for nested namespace declarations to CppParser, otherwise PocoDoc will fail to generate the reference documentation.

mikomikotaishi · 2026-03-12T14:01:39Z

@obiltschnig I have added nested namespace support and a test case now

obiltschnig · 2026-03-12T14:51:37Z

@obiltschnig I have added nested namespace support and a test case now

Great, thank you!

CppParser/include/Poco/CppParser/NameSpace.h

CppParser/src/NameSpace.cpp

matejk · 2026-03-15T09:34:42Z

CppParser/src/Parser.cpp

+				pNS->setInline(inlineFlags[i]);
+			}
+			// Only add the first (outermost) namespace to the global symbol table,
+			// and only if it's not already there


The addToGST logic has subtly changed from the old behavior.

Old code set addGST = undefined (true when the namespace was newly created, false when found via lookup).

New code checks _gst.find(namespaceNames[i]) == _gst.end(), which looks up the simple name in the flat global symbol table.

Since _gst is a multimap, find() will match any symbol with that name (not just namespaces), which could give false negatives if a non-namespace symbol happens to share the name. Consider using the original approach:

pushNameSpace(pNS, -1, (i == 0 && undefined));

This preserves the old semantic: add to GST only when the outermost namespace was newly created.

OK, I'll apply this

matejk · 2026-03-15T09:34:42Z

CppParser/include/Poco/CppParser/NameSpace.h

+	[[nodiscard]]
 	Symbol::Kind kind() const;
 	std::string toString() const;



Nit: [[nodiscard]] was added to isInline() and kind() but not to toString() on the next line. Consider adding it for consistency, or removing from kind() since the existing codebase doesn't use [[nodiscard]] elsewhere in this class.

I probably would have had it added in my other pull request for [[nodiscard]], #5156. If you want I could just move ahead with adding it here, rather than waiting for #5156.

Edit: I think it's a good idea if we move ahead with it here, rather than waiting for #5156, seeing as it's a related functionality that touches this class.

Yes, move this to #5156.

I will keep the one method NameSpace::isInline() marked as [[nodiscard]], as #5156 doesn't have that method, but leave the rest to #5156 as requested, then.

matejk · 2026-03-15T09:34:42Z

CppParser/testsuite/src/CppParserTest.cpp

+}
+
+
 void CppParserTest::setUp()


Good test coverage for the basic cases. Consider adding tests for:

Repeated declaration of the same nested namespace (e.g., namespace A::B { class X; } followed by namespace A::B { class Y; }) to ensure both symbols end up in the same namespace

Single (non-nested) namespace as a regression test for the refactored code

Deeply nested (4+ levels) to verify the pop loop works correctly

OK, added these.

matejk · 2026-03-15T09:38:36Z

Hi @mikomikotaishi, I opened new issues for CodeQL issues to be analysed separately.

Please take a look at the comment for CppParser. Thank you for your contributions.

matejk · 2026-03-15T12:47:20Z

dependencies/tessil/include/Poco/ordered_hash.h

@@ -117,7 +115,7 @@ const T& clamp(const T& v, const T& lo, const T& hi) {
 }

 template <typename T, typename U>
-static T numeric_cast(U value,
+T numeric_cast(U value,


This file lives under dependencies/tessil/ and should not be modified as part of POCO changes — it's a bundled third-party library. If this static removal is needed, it should be done upstream or tracked as a separate dependency patch. Please revert this change.

Non-Poco files were extracted to dependencies on main and this change can't be applied directly.

This is the only way I can get modules to work, since apparently something calls these internally linked symbols.

I'm not actually sure what to do instead here

My proposal:

modify ordered_hash.h in dependencies, Write a note to dependencies/README.md about the change and the reason for the change.

Then open PR in https://github.com/Tessil/ordered-map for the same change to get it included upstream.

OK, I submitted a similar pull request at Tessil/ordered-map#54.

Do note that the repo has a few stale PRs, the most recent one being in May of last year, which haven't been responded to at all, though the last commit to the repo was from the author 4 months ago.

Some stable products change very slowly. We'll see what happens.

matejk · 2026-03-16T18:42:27Z

@mikomikotaishi, please do not force push because the track of changes is then lost. We will squash merge in the end after all of the changes are done.

matejk · 2026-03-16T18:47:37Z

dependencies/cpptrace/src/symbols/dwarf/debug_map_resolver.cpp

@@ -19,8 +19,7 @@
 #include <vector>

 CPPTRACE_BEGIN_NAMESPACE
-namespace detail {
-namespace libdwarf {
+namespace detail::libdwarf {


Revert all changes in cpptrace. We use it unchanges from upstream.

matejk · 2026-03-16T18:48:16Z

dependencies/v8_double_conversion/src/cached-powers.h

-namespace double_conversion {
-
-namespace PowersOfTenCache {
+namespace double_conversion::PowersOfTenCache {


Revert all changes in v8_double_conversion. We use it unchanged from upstream.

mikomikotaishi · 2026-03-16T18:48:31Z

I only use force pushes when it's a small amendment, such as a typo or a minor fix to a broken push.

However that being said I will avoid doing this in the future.

mikomikotaishi · 2026-03-18T16:57:32Z

Sorry for the delay in the response, I didn't see it until now. I've reverted all changes made in dependencies (except for removing static from dependencies/tessil/include/Poco/ordered_hash.h and the notice about this in the readme file).

mikomikotaishi · 2026-03-19T09:21:22Z

CppParser/src/Parser.cpp

+		}

 		if (isOperator(pNext, OperatorToken::OP_ASSIGN))
 		{


I just want to ensure I am not misunderstanding: should this not permit re-defining/re-opening namespaces? In the tests, I re-open namespaces by re-using them, and am not sure if this is the intended behaviour or not.

matejk · 2026-03-19T09:29:16Z

CppParser/src/Parser.cpp

+				// update the inline flag
+				pNS->setInline(inlineFlags[i]);
+			}
+			pushNameSpace(pNS, -1, i == 0);


I think that here it should be:

pushNameSpace(pNS, -1, (i == 0 && undefined))

I just want to ensure I am not misunderstanding: should this not permit re-defining/re-opening namespaces? In the tests, I re-open namespaces by re-using them, and am not sure if this is the intended behaviour or not. You mentioned earlier that you made a mistake in the comment you left, but this was on line 257. Could you give clarity on what the intended behaviour should be, and whether my tests should reuse namespaces?

Re-opening namespaces is perfectly fine and must keep working — that is not the concern here.

The issue is about duplicate entries in _gst (the global symbol table, a std::multimap), not about preventing namespace re-opening.

Old code (before this PR):

NameSpace* pNS = dynamic_cast<NameSpace*>(currentNameSpace()->lookup(fullName)); bool undefined = (pNS == nullptr); if (undefined) pNS = new NameSpace(name, currentNameSpace()); pushNameSpace(pNS, -1, undefined); // addGST = true ONLY when newly created

When namespace Poco appeared a second time in the same file, undefined was false, so the existing Poco namespace was not inserted into _gst again.

Current code:

bool undefined = (pNS == nullptr); // ... pushNameSpace(pNS, -1, i == 0); // addGST = true ALWAYS for outermost

Now every time namespace Poco::Net appears (even the 2nd, 3rd time in the same file), the outermost namespace Poco is re-inserted into _gst. Since _gst is a multimap, this creates duplicate entries.

The fix (i == 0 && undefined) restores the original semantic: add to _gst only when the namespace is seen for the first time. Re-opening still works exactly as before — lookup() finds the existing namespace, symbols get added to it — we just skip the redundant _gst insert.

OK, I'll apply the change then.

Making this change seemed to cause the unit tests to fail, particularly at the line st.find("A") != st.end(). Any ideas on what to do?

My suggestion was actually wrong and should be reverted.

It does not work because there is a global root() singleton which is shared across runs in tests.

A apologise.

OK, I've gone ahead and undid the edit.

Should we wait for the tests to finish before merging, or could we just go ahead and do it knowing that two commits ago the tests all passed?

matejk · 2026-03-19T09:31:12Z

There is potentially one change in CppParser. Please take a look.

Rebase (or merge) from latest main in Poco repository to resolve conflicts.

Then we are ready for merge.

mikomikotaishi force-pushed the concatenate-namespace-declarations branch from 52b7231 to 47fbbd2 Compare March 9, 2026 14:42

Use C++17 nested declarations, closes pocoproject#5234

57d3f32

mikomikotaishi force-pushed the concatenate-namespace-declarations branch from 47fbbd2 to 57d3f32 Compare March 9, 2026 15:13

Add inline namespace parsing to CppParser

3ba6402

mikomikotaishi force-pushed the concatenate-namespace-declarations branch from 23cd29b to 3ba6402 Compare March 12, 2026 07:04

mikomikotaishi changed the title ~~Use C++17 nested declarations, closes #5234~~ Use C++17 nested declarations and add CppParser support, closes #5234 Mar 13, 2026

matejk mentioned this pull request Mar 15, 2026

Address pre-existing CodeQL alerts surfaced by PR #5235 #5243

Open

matejk reviewed Mar 15, 2026

View reviewed changes

mikomikotaishi force-pushed the concatenate-namespace-declarations branch from 0147bc1 to 2cb49b6 Compare March 15, 2026 11:33

matejk mentioned this pull request Mar 15, 2026

chore(codeql): address pre-existing CodeQL alerts #5243 #5244

Open

3 tasks

mikomikotaishi force-pushed the concatenate-namespace-declarations branch from 2cb49b6 to e5b887d Compare March 15, 2026 12:09

matejk reviewed Mar 15, 2026

View reviewed changes

Apply recommended changes

59d9112

mikomikotaishi force-pushed the concatenate-namespace-declarations branch from 1694d69 to 59d9112 Compare March 16, 2026 02:59

Merge branch 'main' into concatenate-namespace-declarations

8c0f57a

mikomikotaishi force-pushed the concatenate-namespace-declarations branch from 43c3784 to 8c0f57a Compare March 16, 2026 04:14

matejk reviewed Mar 16, 2026

View reviewed changes

Undo all changes made in dependencies

713c4ef

mikomikotaishi commented Mar 19, 2026

View reviewed changes

matejk reviewed Mar 19, 2026

View reviewed changes

mikomikotaishi added 3 commits March 19, 2026 18:43

Merge branch 'pocoproject:main' into concatenate-namespace-declarations

3d73131

Check for undefined in pushNamespace() call

a3f451e

Undo the last commit

67069dc

Conversation

mikomikotaishi commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikomikotaishi commented Mar 9, 2026

Uh oh!

mikomikotaishi commented Mar 9, 2026

Uh oh!

obiltschnig commented Mar 11, 2026

Uh oh!

mikomikotaishi commented Mar 12, 2026

Uh oh!

obiltschnig commented Mar 12, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikomikotaishi Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matejk commented Mar 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikomikotaishi Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikomikotaishi Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matejk commented Mar 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikomikotaishi commented Mar 16, 2026

Uh oh!

mikomikotaishi commented Mar 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikomikotaishi commented Mar 9, 2026 •

edited

Loading

mikomikotaishi Mar 15, 2026 •

edited

Loading

mikomikotaishi Mar 15, 2026 •

edited

Loading

mikomikotaishi Mar 16, 2026 •

edited

Loading