This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Move unexecuted test block into it's own source file
ClosedPublic

Authored by isuckatcs on Oct 14 2022, 9:08 AM.

Details

Summary

Inside lambdas.cpp a block of code wasn't executed,
because they required the standard to be at least
-std=c++14. This patch moves this block of code into
it's own source file.

Diff Detail

Event Timeline

isuckatcs created this revision.Oct 14 2022, 9:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 14 2022, 9:08 AM
xazax.hun accepted this revision.Oct 14 2022, 9:09 AM
This revision is now accepted and ready to land.Oct 14 2022, 9:09 AM
isuckatcs updated this revision to Diff 467793.Oct 14 2022, 9:10 AM

Added missing '\n' to the end of the new file.

isuckatcs updated this revision to Diff 467810.Oct 14 2022, 9:46 AM

Moved clang_analyzer_eval() out of the lambda body, because no diagnostic was reported from the inside.

Moved clang_analyzer_eval() out of the lambda body, because no diagnostic was reported from the inside.

Interesting. Do you know why is that the case?

Moved clang_analyzer_eval() out of the lambda body, because no diagnostic was reported from the inside.

Interesting. Do you know why is that the case?

I quickly looked at the source code of the said checker and I found this:

void ExprInspectionChecker::analyzerEval(const CallExpr *CE,
                                         CheckerContext &C) const {
  const LocationContext *LC = C.getPredecessor()->getLocationContext();

  // A specific instantiation of an inlined function may have more constrained
  // values than can generally be assumed. Skip the check.
  if (LC->getStackFrame()->getParent() != nullptr)
    return;

  reportBug(getArgumentValueString(CE, C), C);
}

I guess the parent stack frame of the lambda is the function in which it's declared, so the bug reportion is skipped.

xazax.hun accepted this revision.Oct 14 2022, 11:14 AM

Oh, I see, this look deliberate. I wonder if we want to add an exception for lambdas though but that would be a different discussions unrelated to this PR.

Oh, I see, this look deliberate. I wonder if we want to add an exception for lambdas though but that would be a different discussions unrelated to this PR.

We could also consider reporting a warning or an error in this case to let the caller know that the function doesn't work in this context. Right now it's up to the caller to figure it out on their own, and if they don't see the report they might think there's something wrong with some other part of their code.

We could also consider reporting a warning or an error in this case to let the caller know that the function doesn't work in this context. Right now it's up to the caller to figure it out on their own, and if they don't see the report they might think there's something wrong with some other part of their code.

Good point! Such a warning would be really helpful.

Herald added a project: Restricted Project. · View Herald TranscriptOct 20 2022, 5:15 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript