[cfe-commits] [PATCH] Implicit fall-through between switch labels

[Jordy Rose]

>> FWIW, Richard convinced me that this must be spelled 'clang::fallthrough' for the immediate future. We should probably focus on that form.
> 
> Done. It turned out though, that namespaces for attributes were not used until now, so I implemented my own vision: I just add namespace  as a prefix to spelling with triple underscore delimiter. Better ideas?

The right thing might be to implement real namespacing now, but why not just use ::, as in the written form? The parser only expects attributes to be made of identifiers, but our table can use any characters we want.

My other suggestion for cramming two values in one field is a null character, but that might have weird interactions with some of our current infrastructure.


> +  SmallVector<char, 20> Buf;
> +  if (ScopeName)
> +    AttrName = (ScopeName->getName() + "___" + AttrName).toStringRef(Buf);
> +

I had to figure out this is building a Twine. It's probably not a big issue, but I would probably change this to use SmallString (basically a nice specialization of SmallVector for char) and += all the pieces.


> +    typedef std::vector<const Stmt*> StmtVec;
> +    bool GetFallThroughSourceStmts(const CFGBlock &B, StmtVec &Unannotated,
> +                                   StmtVec &Annotated) {

Should this be a SmallVector? The argument would then be a SmallVectorImpl<const Stmt*>, and only the actual stack-allocated vector needs to have a size.

Also, do we need the annotated set? If not, we could leave it out for now, and only put it back in if we want to try for some intelligence in the suggestions.


> +      SourceLocation L = Label->getLocStart();
> +      S.Diag(L, diag::note_insert_fallthrough_fixit) <<
> +        FixItHint::CreateInsertion(L, "[[clang::fallthrough]];");

This will give a proper diagnostic, but not a very nice placement of the attribute. Clients like Xcode actually apply fixit hints to the user's code, and really we want this on a previous line. Can we figure out how to insert this in the right place, rather than suggest something the user will have to manually edit afterwards?

Jordy