[PATCH] D72484: [clang-tidy] Fix check for Abseil internal namespace access

EricWF closed this revision. EricWF added a comment. Committed in 020ed6713d889a95f8c98d7725c87b458d99f6b3 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72484/new/ https://reviews.llvm.org/D72484

[PATCH] D72484: [clang-tidy] Fix check for Abseil internal namespace access

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM Comment at: clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp:43-44 + + if (!LocAtFault.isValid()) +return; + EricWF wrote: > rogeeff wrote: > > aaron.ballman wrot

[PATCH] D72484: [clang-tidy] Fix check for Abseil internal namespace access

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM. The weird template test case might is derived from an old failure that I believe was addressed in Clang, but regression tests are fine by me. The macro expansion fix is correct and corre

[PATCH] D72484: [clang-tidy] Fix check for Abseil internal namespace access

rogeeff marked 3 inline comments as done. rogeeff added inline comments. Comment at: clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp:43-44 + + if (!LocAtFault.isValid()) +return; + aaron.ballman wrote: > rogeeff wrote: > > lebedev.ri wro

[PATCH] D72484: [clang-tidy] Fix check for Abseil internal namespace access

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp:43-44 + + if (!LocAtFault.isValid()) +return; + rogeeff wrote: > lebedev.ri wrote: > > Is there test coverage for this? When does/can this h

[PATCH] D72484: [clang-tidy] Fix check for Abseil internal namespace access

rogeeff marked 2 inline comments as done. rogeeff added inline comments. Comment at: clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp:43-44 + + if (!LocAtFault.isValid()) +return; + lebedev.ri wrote: > Is there test coverage for this? Whe

[PATCH] D72484: [clang-tidy] Fix check for Abseil internal namespace access

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp:43-44 + + if (!LocAtFault.isValid()) +return; + Is there test coverage for this? When does/can this happen? CHANGES SINCE LAST ACTION https

[PATCH] D72484: [clang-tidy] Fix check for Abseil internal namespace access

Eugene.Zelenko added a comment. In D72484#1817187 , @rogeeff wrote: > It is my first time submitting clang-tidy change. So I'm not sure of the > procedure exactly. Do we wait for something from me? Reviewers should make comments and finally approve patc

[PATCH] D72484: [clang-tidy] Fix check for Abseil internal namespace access

rogeeff added a comment. It is my first time submitting clang-tidy change. So I'm not sure of the procedure exactly. Do we wait for something from me? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72484/new/ https://reviews.llvm.org/D72484 ___

[PATCH] D72484: [clang-tidy] Fix check for Abseil internal namespace access

Eugene.Zelenko added a comment. Please mark fixed comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72484/new/ https://reviews.llvm.org/D72484 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

[PATCH] D72484: [clang-tidy] Fix check for Abseil internal namespace access

rogeeff updated this revision to Diff 237225. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72484/new/ https://reviews.llvm.org/D72484 Files: clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-fi

[PATCH] D72484: [clang-tidy] Fix check for Abseil internal namespace access

Eugene.Zelenko added a comment. Should be change mentioned in Release Notes? Comment at: clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp:40 - diag(InternalDependency->getBeginLoc(), + auto loc_at_fault = + Result.SourceManager->getSpellingLoc(Inter