Page MenuHomePhabricator

Revert "Remove false positive in AvoidNonConstGlobalVariables."
Needs ReviewPublic

Authored by vingeldal on Apr 29 2020, 12:10 PM.

Details

Summary

There was concernes about a false positive in clang-tidy check
AvoidNonConstGlobalVariables after it was merged and a fix for
assumed false positive was submitted in:
https://reviews.llvm.org/D77461

There was also some discussion regarding if this was actually a
false positive or a true positive. An issue was submitted to the
C++ Core Guidelines asking for a clarification in the matter:
https://github.com/isocpp/CppCoreGuidelines/issues/1599

The answer from the C++ Core Guidelines project seems to go
against the previous assumption that this was a matter of a
false positive, so this patch is suggestion to revert the
previous fix of the assumed false positive.

This reverts commit b2d8c89ea48beb83e0392b1f00c9eafa33c09ca8.

Diff Detail

Event Timeline

vingeldal created this revision.Apr 29 2020, 12:10 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 29 2020, 12:10 PM

I only see a comment on the issue, but not that the issue was resolved in favor of that comment (unless I've missed something). If the issue gets resolved in the direction that comment is going, then the revert makes sense (though we should add an explicit test case showing that we want the diagnostic).

I only see a comment on the issue, but not that the issue was resolved in favor of that comment (unless I've missed something). If the issue gets resolved in the direction that comment is going, then the revert makes sense (though we should add an explicit test case showing that we want the diagnostic).

No, your not missing anything. The issue isn't actually closed yet. I'll remember to update tests as appropriate if the issue is resolved in favor of this revert. Until the issue is resolved I'll just wait.

The issue is resolved now in the CppCoreGuidelines.