[cfe-dev] Who's your daddy?

[Csaba Raduly]

Hi,

In our project (which can be built with either GCC or clang) we have
the convention that every 'if' must have an 'else'; this way we can
use line coverage to simulate branch coverage. We have a macro that
can be added at the end of an if statement:

#define ELSE_NOTHING_TO_DO else { asm("nop"); }

Now I'm writing a clang plugin that emits a warning when it sees an if
without an else. However, some if statements should get an exemption:

{
  if (....) {
    return;
  }

  another statement;
}

An 'if' statement whose 'then' clause ends with a
return/goto/break/continue/throw, and has a "next statement", doesn't
need an else because the coverage of the next statement can be used to
verify that both branches have been taken.

It's the "next statement" which is a bit tricky, because I need to get
the parent of the 'if' to check if it's a compound statement. I
couldn't figure out how to get a ParentMap inside VisitStmt(), so I
came up with the following instead:

class IfCheckVisitor : public RecursiveASTVisitor<IfCheckVisitor>
{
private:
    ASTContext *context;
    std::unique_ptr<ParentMap> pm;

public:
   bool VisitDecl(Decl *d) {
        switch (d->getKind()) {
        case Decl::Function: {
            FunctionDecl *fd = static_cast<FunctionDecl *>(d);

            SourceManager & sm = context->getSourceManager();
            SourceRange    const& r = d->getSourceRange();
            SourceLocation const& b = r.getBegin();
            if (!sm.isInMainFile(b))
                return true; // avoid spamming for headers

            if (fd->hasBody()) {
                pm.reset(new ParentMap{fd->getBody()});
            }
            //else { // must be a forward decl of a function. keep quiet
            //}
            break; }
        default:
            break;
        }
        return true;
    }

    bool VisitStmt(Stmt *s) {
        switch (s->getStmtClass()) {
            case Stmt::IfStmtClass: {
                IfStmt const *iff = static_cast<IfStmt*>(s);
                // if it has 'else', return
                // if the 'then' is not break/continue/return/..., complain

                Stmt const *ifparent = pm->getParent(iff);
                // if no "next" inside ifparent, complain
                break;
            } // case IfStmt

            default:
                break;
        } // switch statement class
  }
};

Is this a good idea? Is it guaranteed that the right ParentMap will be
active when I get calls to VisitStmt (i.e. when a statements is
visited, the last function declaration is the function the statement
is in)?

Csaba
-- 
GCS a+ e++ d- C++ ULS$ L+$ !E- W++ P+++$ w++$ tv+ b++ DI D++ 5++
The Tao of math: The numbers you can count are not the real numbers.
Life is complex, with real and imaginary parts.
"Ok, it boots. Which means it must be bug-free and perfect. " -- Linus Torvalds
"People disagree with me. I just ignore them." -- Linus Torvalds