Repository navigation
Remove shadowing warnings from testlib.h - #237
Merged
Merged
Conversation
Unit tests for the code whose parameters shadow members or globals: pattern::src() keeps the pattern as given while matching ignores spaces, the pattern error message shows the text without spaces, a rejected setAppesModeEncoding() leaves the current value unchanged, both InStream::init() overloads store the mode before the file is opened, and the constructors of ValidatorBoundsHit, exit_exception and the file readers keep their arguments. The tests pass on the current header; refs gain only the new lines. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With -Wshadow every program that includes testlib.h got 13 warnings (14 with TESTLIB_THROW_EXIT_EXCEPTION_INSTEAD_OF_EXIT) from the header itself: g++ -Wshadow, clang++ -Wshadow/-Wshadow-all and MSVC C4459 (and C4458 once its pragma is lifted). Parameters named like a member or the global appesModeEncoding are renamed: - pattern::matches(s), pattern(s) -> str, ptrn - FileInputStreamReader::getc(file) -> f - reader constructors (file, name) -> f, fileName - ValidatorBoundsHit(minHit, maxHit) -> lower, upper - InStream::init(..., mode) -> type - setAppesModeEncoding(appesModeEncoding) -> value - exit_exception(exitCode) -> code Only parameter names change: fields, signatures, default arguments and pragmas stay as they were, and every new name already occurs in testlib.h, so no user macro can break the header that did not break it before. The generated code is byte-identical to the previous header (g++, clang++ and MSVC, optimised and not). The warnings were reported in PR #228 and issue #229. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
test-000 compiles checkers, validators, generators and interactors with shadowing diagnostics as errors: -Wshadow for GCC, -Wshadow-all for Clang (-Wshadow where it is unknown), -we4456/7/8/9 for MSVC through the CL variable, as tests/scripts/compile does not pass CPP_OPTS to cl.exe. Other .cpp files keep the usual flags. test-011_shadow compiles probes that instantiate the templates of testlib.h (MSVC checks templates only where they are instantiated), with and without TESTLIB_THROW_EXIT_EXCEPTION_INSTEAD_OF_EXIT, and the signatures of the renamed functions. On MSVC it also checks testlib.h against C4458 with a copy that lacks the pragma disabling it. Fixtures make sure shadowing in a program's own code is still diagnosed, both as an error and as a warning, so the flags are known to work; the member fixture pins that MSVC C4458 stays disabled after testlib.h. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MSVC checks the body of a template where the template is instantiated, that is after the program's own code. A local such as `T value` in optValueToIntegral() then hides a global `value` of the program and /W4 reports C4459 inside testlib.h; GCC and Clang check where the template is defined and are not affected. Common program globals such as i, n, s, x, d, size, first, result and value collided with 60 locals in 20 function templates and with `ap` of FMT_TO_RESULT. Those locals are now named `_i`, `_value` and so on (`n_`, `sum_` become `_n`, `_sum`). Names that begin with an underscore are reserved in the global namespace, so no valid program declares such a global; none of them is a macro of the system headers. Template parameters are checked where they are declared, explicit specializations are ordinary functions, and neither needs this. The generated code is unchanged apart from the symbol of the static local stringstream in vtos(), which is named after the variable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fixture-user-globals.cpp declares globals named like the former locals of the templates and like common program variables, then instantiates the templates; MSVC must report nothing in testlib.h (it reported 59 C4459 before the previous commit). shadow-names.cpp checks that none of the new "_x" names is a macro after testlib.h on each compiler. The locals of shadow-probe.cpp are renamed so that the fixture can include it after its globals. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Clang with delayed template parsing, the default for the MSVC target before C++20, checks a template where it is instantiated, parameters included: vtos(const T &t) is reported by -Wshadow when the program has a global t. The parameters of the function template definitions get the "_" prefix, like their locals; the locals n_ and sum_ that copied such parameters become _rest and _total. Declarations without a body and explicit specializations keep their names, as do the pre-C++11 branches, which no supported compiler sees. The generated code is unchanged. fixture-user-globals.cpp now also declares the former parameter names, and shadow-names.cpp checks the new names. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A check that inserts a shadowing canary into every function template showed four live templates the probe did not instantiate on MSVC: println(const A *, const A *) (int* arguments pick the range overload), opt<T>(index) for floating point and unsigned types, and format(const std::string, ...). MSVC checks a template body only where it is instantiated, so the probe calls them now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
_points read like the TResult value of the same name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
It was the last parameter of a function template without the "_" prefix. No program global can be named format, so it was never reported, but it broke the rule stated in testlib.h. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
In C++23 every call of the probe with two or more arguments went to the Args&& overloads: only const lvalues reach the const& ones. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Clang reports a template parameter that hides a program global only with delayed template parsing, the default just for the MSVC target before C++20. So nothing checked the parameters of the C++20 format() and C++23 println() templates: without their "_" prefix test-011 still passed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…efinition The definition's parameters became _text and _nonnegative, and clang-tidy (readability-inconsistent-declaration-parameter-name) started to report the declaration, which still said s and nonnegative. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Nov 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Programs that include testlib.h got shadowing warnings from the header itself with g++
-Wshadow, clang++-Wshadow/-Wshadow-alland MSVC/W4(C4456–C4459). Reported in #228 and #229.appesModeEncodingare renamed (13 places, plus one more withTESTLIB_THROW_EXIT_EXCEPTION_INSTEAD_OF_EXIT). Every new name already occurs in testlib.h.int value;,n,s,t, ... therefore got C4459/-Wshadowinside testlib.h. Locals and parameters of function template definitions now have a_prefix (_value,_n, ...). Such names are reserved in the global namespace, and a test checks that none of them is a macro on the CI platforms.#pragma warning(disable: 4458)are unchanged.Tests:
test-000 compiles checkers, validators, generators and interactors with shadowing diagnostics as errors (
-Wshadow,-Wshadow-all,-we4456 -we4457 -we4458 -we4459).New test-011_shadow:
TESTLIB_THROW_EXIT_EXCEPTION_INSTEAD_OF_EXIT;int valueamong them;It fails on the old header and passes on the new one.
test-004: unit tests that pin the behaviour of the touched functions.
Generated code: for every commit, the code built with the old and the new header was compared. It is byte-identical on g++ 7–15, clang 3.5–22 (Linux, macOS, Windows, Docker) and MSVC 2022/2026 x86/x64, with and without optimisation, in several standards. The comparison ignores only two things: MSVC lambda hash names and the symbol of the renamed static local in
vtos(). It detected deliberately broken renames. One difference was seen once on VS2026 x64 (/O2, C++20, a single test file); it did not reproduce in four reruns.CI: 25/25 on 70a17b4.
Limitations:
_ior_value, defined before#include "testlib.h", would break the build: it would replace a parameter or a local of a testlib template. Such macros must not be defined before including testlib.h.int TResult;,int sort;) still breaks the compilation of the templates.🤖 Generated with Claude Code