ping :)
@njames93 I added more CHECK-FIXES and CHECK-FIXES-NOT statements in the tests.

• addded CHECK-FIXES in clang-tidy tests
• merged latest main into branch

friendly ping :) (maybe @njames93 ?)

• fix documentation reference

Thank you for the review! I adjusted the patch.

• addressing review comments and remove unrelated changes

• fix: remove clangAnalysis link in cppcoreguidelines and add it in misc

• refactor: rename check to 'misc-const-correctness' and adjust the tests accordingly
• docs: adjust release notes and adjust check docs slightly

Sorry for the long delay, i simply forgot.
The patch is commited! :)

ok. then i will continue under misc-const-correctness. thank you for the clarifications!

Sorry, for my own sanity, I've stopped reviewing C++ Core Guideline reviews for their diagnostic behavior until the guideline authors put effort into specifying realistic enforcement guidance.

ping @aaron.ballman @njames93 :)

LGTM, but please adjust the naming nit to a better name.

won't happen anymore realistically.

LGTM with a small nit.

Another full run on LLVM with:

• address reviews comments, part 1
• Merge branch 'main' into feature_rebase_const_transform_20210808
• fixing iterator-to-bool conversion and addressing other more review comments

In D54943#3292552, @0x8000-0000 wrote:

@aaron.ballman - can this land for Clang14, or does it have wait for 15?

Are there any other reviewers that can approve this?

ping @aaron.ballman just in case the patch is now down on the list, I am sorry for my high latency :(

• use boolean for options
• fix snafoo on arc usage

• use boolean for option parsing

• fix: adjust unit test for new array-pointer condition in range-based for loops

• fix release note ordering

• improve documentation a bit

addressed all outstanding review comments.

• Merge branch 'main' into feature_rebase_const_transform_20210808
• fix: address other review comments
• remove one false positive with pointers in a range-based for loop on arrays

• Merge branch 'main' into feature_rebase_const_transform_20210808
• fix: fixes for most review comments

In D54943#3194643, @aaron.ballman wrote:

Thanks for pinging on this, it had fallen off my radar! @JonasToth -- did you ever get the chance to talk with @cjdb about the overlap mentioned in https://reviews.llvm.org/D54943#2950100? (The other review also seems to have stalled out, so I'm not certain where things are at.)

@aaron.ballman ping for review :)

ping

I analyzed the false positive and its probably not so simple to fix.

• add reproducer for '__unaligned' bug

In D54943#2956218, @tiagoma wrote:

I am getting false positives with

struct S{};

void f(__unaligned S*);

void scope()
{
	S s;
	f(&s);
}

• Merge branch 'main' into feature_rebase_const_transform_20210808

• minor adjustments for comments

thanks for your testing! i will look at the __unaligned issue, not sure if clang supports it, its an MSVC extension, is it?

In D54943#2947902, @cjdb wrote:

@JonasToth, @aaron.ballman suspects that there's enough overlap between this patch and D107873 to consider merging the two efforts. I'm happy to collaborate, but we should probably check that our goals are aligned. Are you active on the LLVM Discord? We could probably discuss a fair bit of this offline and then ping Aaron after we work out whether or not to do this.

The check in the latest form can properly transform llvm/lib and clang/ and requires 28 manual fixes afterwards (values and references are transformed, no other modifications).
Some are good and not an issue with the analysis itself, some may or may not be an issue with the analysis itself. I think it is more a template meta programming thing.

• fix: auto and type-dependent initializers fixed by ignoring them correctly in the matcher
• fix docs on limitations
• fix: deduplicate diagnostics from template instantiations

• write documentation for const-analysis check

• Merge branch 'main' into feature_rebase_const_transform_20210808

In D54943#2944680, @0x8000-0000 wrote:

Using the checker now in our production BuildBot - no crashes and no false positives. Can't say if there are false negatives, but at any rate the checker is better than most colleagues at finding what should be declared const.

Whats left from my personal todo is adjusting the documentation.
I think then this check can work as linter and if you want as fixer as well, but this has to be enable explicitly.

• remove transformation as default option, now it must be activated by the user explicitly
• register only for C++
• add a test-case for VLAs

• retry upload of patch to be a diff to main

• rebase to master
• fix a crash where a scope matched but non of its variables, leading to nullptr dereference (found with the current test-suite)
• remove outcommenting of tests that showed undesirable behavior

In D54943#2633408, @tiagoma wrote:

Can we get this in? I work in Microsoft Office and we have been using this checker and it works great! There are a couple of issues with it and I would like to contribute fixes.

LGTM! thanks for fixing.

Hey Alex, first thanks for the bug fixing! Of course we are interested in improvements in clang-tidy, but i think noone is actually employed to help out here and the normal reviewers have many, many reviews parallel. Sometimes stuff just slips through :/

You can land it. Worst case would be post-commit comments, but this is not a complicated patch and should be fine!

I am fine now, thank you!

The issue is resolved now in the CppCoreGuidelines.

The context thingie was not that important here, but still thanks :)

LGTM.
Short reminder to please use full context patches. Its not an issue right now, but review sometimes just needs the context to understand the changes :)

• fix test RUN line

• no delayed template parsing
• adjust release note issue

• missed one place of decltype

• fix platform dependent warning message for decltype to just match the beginning and not the 'a.k.a'

• address review comments

• update to landed ExprMutAnalyzer changes

• [Feature] add elvis operator detection

• fix imprecisions

In D54943#2302315, @0x8000-0000 wrote:

In D54943#2292377, @0x8000-0000 wrote:

In D54943#2291969, @JonasToth wrote:

@AlexanderLanin @0x8000-0000 i created the branch release-11-const (https://github.com/JonasToth/llvm-project/tree/release-11-const) in my fork.

This branch is based on 11-rc3 and cherry picks the commits that correspond to this revision.
I hope this is of any use for you! I will probably forget to update this branch all the time, but if I feel like there has been actual progress made, i will update!

Thank you Jonas - will add this to our CI tool for the new project and try to find some time to run it on the legacy project.

Just a quick update; in clang-tidy mode it works great, with no false positive and no crashes. I haven't tried the 'apply-replacements' mode.

rebase to newest expmutanalyzer patch

@aaron.ballman I update the code a bit. This stuff is just a hydra :/

• document sections in the testcases, hopefully little more structure for comprehension
• fix derived-to-base-cast OUTSIDE of an conditional operator
• improve type-dependent callexpr, too
• better canResolveToExpr matcher code
• adjust matcher for comma

I am confused now, but richard knows this stuff very well! The patches you referenced showed only cases where the static was defined in an inline-header definition (and templates). Does this make a difference on the semantics?
This should be clear before we continue.

In D54943#2295894, @AlexanderLanin wrote:

Off-Topic: I was just attempting to apply this to my codebase at work.
Seems this will be more challenging than anticipated 😉
My best guess is this is related to D72730

Applying fixes ...
terminate called after throwing an instance of 'std::bad_alloc'
  what():  std::bad_alloc

In D54222#2294123, @jranieri-grammatech wrote:

In D54222#1290789, @JonasToth wrote:

I agree with @Eugene.Zelenko that this check should rather live in bugprone- as the issue its diagnosing is not LLVM specific. It is ok to add an alias into the llvm module.

Here are the warnings it issues with the patch's matcher:

clang/include/clang/StaticAnalyzer/Core/CheckerManager.h:625:43: warning: address of static local variable 'tag' may not be identical across library boundaries [llvm-problematic-statics]
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:219:1: warning: address of static local variable 'Index' may not be identical across library boundaries [llvm-problematic-statics]
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h:25:1: warning: address of static local variable 'Index' may not be identical across library boundaries [llvm-problematic-statics]

And here are the additional warnings it issues when relaxing the matcher (to be unless(isExpansionInMainFile())):

clang/lib/StaticAnalyzer/Checkers/Iterator.h:130:47: warning: address of static local variable 'Index' may not be identical across library boundaries [llvm-problematic-statics]
clang/lib/StaticAnalyzer/Checkers/Iterator.h:136:47: warning: address of static local variable 'Index' may not be identical across library boundaries [llvm-problematic-statics]
clang/lib/StaticAnalyzer/Checkers/Iterator.h:142:47: warning: address of static local variable 'Index' may not be identical across library boundaries [llvm-problematic-statics]
clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h:60:5: warning: address of static local variable 'index' may not be identical across library boundaries [llvm-problematic-statics]
llvm/utils/unittest/googletest/src/gtest.cc:3989:3: warning: address of static local variable 'instance' may not be identical across library boundaries [llvm-problematic-statics]

And also this one, which was hidden by default but made running tidy noisier by always saying there was a hidden warning:

/usr/bin/../include/c++/v1/functional:1960:9: warning: address of static local variable '__policy_' may not be identical across library boundaries [llvm-problematic-statics]

All of the first set of warnings might be actual problems, but the second set definitely aren't problems because they aren't going to be used across shared library boundaries. By making this be a LLVM-specific check, we can avoid the noise of these false positives. Also, I looked through the C++ Core Guidelines for any rules that might address this but didn't see anything relevant.

Thoughts?

• fix typo that provided wrong argument to AST building

• address review comments