================
@@ -231,6 +231,11 @@ StmtResult
Parser::ParseStatementOrDeclarationAfterAttributes(
GNUAttrs.Range.getBegin());
} else if (GNUAttrs.Range.getBegin().isValid())
DeclStart = GNUAttrs.Range.getBegin();
+ // A declaration that declares nothing (`int;`) yields no Decl but still
+ // occupies the statement position; unlike a pragma, ParseStatement()
must
+ // not skip it.
+ if (!Decl)
+ return Actions.ActOnNullStmt(PrevTokLocation);
----------------
AaronBallman wrote:
I'm not certain an empty `DeclStmt` will cover all of the scenarios because I
don't think we create a `DeclStmt` for a field declaration, but that's another
case we have to care about. I think this is a full list of the situations we
need to handle if we want to retain an AST node for something which declares
nothing: https://godbolt.org/z/qrfdffaY5
Personally, I think there are two reasonable ways forward:
1) Continue to not retain any AST node for these (note, the empty anon struct
in C makes a `RecordDecl` and in C++ makes both a `CXXRecordDecl` and `VarDecl`
already) and fix the issue in CodeGen for the narrow case in #215454
2) Create an empty `DeclStmt`, `FieldDecl`, etc in the AST and change all the
fallout.
My preference is for (1) because I don't think the amount of changes for (2)
are worth it for such a nominal language extension. In fact, someday I'd prefer
for us to stop supporting this as an extension. In C++ mode it's pretty easy to
do I think (GCC already covers this only under `-fpermissive` which Clang will
never support, so we can already justify turning this into a warning which
defaults to an error in C++). I suspect it's harder to do in C, but I also
question how often this extension is used intentionally. Some searches show it
does get turned off by a few hundred projects:
https://sourcegraph.com/search?q=context:global+-file:.*clang.*+-file:.*test.*+lang:Makefile+-Wno-missing-declarations&patternType=keyword&sm=0
https://sourcegraph.com/search?q=context:global+-file:.*clang.*+-file:.*test.*+lang:CMake+-Wno-missing-declarations&patternType=keyword&sm=0
but how much of that is cargo cult and the diagnostic never actually triggers
vs intentionally creating declarations which declare nothing.
All that said, I'm not certain @efriedma-quic or others have a different take
on the situation. The one thing I'm not comfortable with is switching these
from no node to a `NullStmt` node; I think that's heading in the wrong
direction because there is no `NullStmt` there. If we had a `RecoveryDecl`
similar to `RecoveryExpr`, that might be more reasonable, but that's basically
the same amount of churn as (2) I believe (if not more).
https://github.com/llvm/llvm-project/pull/224682
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
