- User Since
- Aug 23 2021, 6:51 AM (9 w, 5 h)
Sat, Oct 23
Fri, Oct 22
Thanks for the review.
Are you able to commit for me?
Pre-merge checks failing because patch cannot be applied.
Therefore recreating patch to fix this.
A collection of minor corrections that I don't think deserve separate commits.
Fix spelling and grammatical mistakes found across the .rst files.
Standardize to US English spelling for words such as "behavior" and "specialization".
Remove repeated words, e.g. "for a a larger user input".
Standardize spelling of "e.g." and "i.e."
Replace curly quote characters (‘ ’ “ ”) with standard straight ones (' ").
Replace en-dash (–) with standard hyphen (-).
In both cases, the latter is used more often in the doc files and is easier to type.
Change double spaces after commas and periods to single space.
Wed, Oct 20
Tue, Oct 12
Sun, Oct 10
Thanks for all the changes. You've addressed all my questions ad I don't have anything more to add. Let's wait for the reviewers to take a look.
Thanks! Simplifying it right down looks to me like the right thing to do.
Sat, Oct 9
The functions that we have been discussing in this patch only run when Clang-Tidy has detected a check violation and is about to print a warning about it. The NOLINT comment checks happen at the last minute. Since foo and bar are not real checks, Clang-Tidy won't find any violations, and so the NOLINT checks won't get the opportunity to run.
Fri, Oct 8
What should happen with the following code:
Thu, Oct 7
Tue, Sep 28
You can see in the attached file, that when I ran the unit test on a x86_64 Windows 10 PC, the diagnostic from error_in_include.inc is printed at the end. However, on this this clang-s390x-linux-multistage build, it is printing the diagnostic from error_in_include.inc first:
Resolving build error due to failed unit test.
Yes, I will need your help to commit.
Also, the pre-merge build failure is confirmed to be a problem with the build system and not the contents of my patch.
Addressing review comments.
Sep 24 2021
Pre-merge build error doesn't seem related to my change, but rebasing patch anyway just to be sure.
Put NOLINTBEGIN comments into 2 buckets:
- Comments that apply to a specific check - NOLINTBEGIN(check)
- Comments that apply to all checks - NOLINTBEGIN(*) and NOLINTBEGIN with no args
If a check is begun with one type of comment, it must be ended with the same type.
Sep 21 2021
- If the search through the preceding lines returns no active NOLINTBEGINs, carry on reading the rest of the file anyway.
Updated according to review comments.
- test/clang-tidy/infrastructure/nolintbeginend-begin-at-eof.cpp: new test
- test/clang-tidy/infrastructure/nolintbeginend-end-at-sof.cpp: new test
- Use llvm::ErrorOr return type with getFile()
- Split diagnostic message into 2 messages (one specific to NOLINTBEGIN and one specific to NOLINTEND)
- if ... else brace wrapping
- Spelling corrected to "nonexistent"
- Assert that Unused diagnostic container is empty
- change std::vector<ClangTidyError> to llvm::SmallVectorImpl
Sep 19 2021
Minor update to function signatures
- Remove arguments that are not absolutely required
- Added consts
Sep 18 2021
- Changed to take a container of diagnostics as an argument. If it finds any stray NOLINTBEGIN/NOLINTEND markers, it creates a diagnostic regarding the stray marker and places it in the container.
Sep 12 2021
It seems Google have implemented NOLINTBEGIN and NOLINTEND support in cpplint. I see lines such as // NOLINTBEGIN(whitespace/line_length), // NOLINTBEGIN(readability/check), // NOLINTBEGIN(build/include) scattered across different Google projects on GitHub.
Changes in this latest patch:
- LineIsMarkedWithNOLINT(): Moved NOLINTBEGIN/END aspects into a separate function.
- LineIsWithinNOLINTBEGIN(): A NOLINTBEGIN/END region is only considered valid if both the BEGIN and END markers are present. If the region is malformed, the original warning will be raised.
Sep 10 2021
Sep 8 2021
- Added test to check what happens when NOLINTEND marker is used before NOLINTBEGIN marker (class B1, line 7).
- Warning is still displayed (NOLINTEND ends suppression but this is redundant as there was no suppression in the first place).
Sep 2 2021
Add additional checks in test/clang-tidy/infrastructure/nolintbeginend.cpp to check that error suppressions in one file are carried over when the file is #included in another file.