This is an archive of the discontinued LLVM Phabricator instance.

Don't reject tag declarations in for loop clause-1
ClosedPublic

Authored by aaron.ballman on Dec 3 2020, 7:33 AM.

Details

Reviewers
rsmith
rjmccall
Summary

We currently reject this valid C construct by claiming it declares a non-local variable: for (struct { int i; } s={0}; s.i != 0; s.i--) ;

The problem is that we expect all declaration in the clause-1 declaration statement to be VarDecls of local variables, but there can be other declarations involved such as a tag declaration. We now ignore tag declarations when deciding whether the clause-1 declarations are valid or not. This fixes PR35757.

Diff Detail

Event Timeline

aaron.ballman requested review of this revision.Dec 3 2020, 7:33 AM
aaron.ballman created this revision.

Please test that there's actually an object declared and that it's not *just* a tag declaration.

Updating based on review feedback.

Please test that there's actually an object declared and that it's not *just* a tag declaration.

Good catch, I've corrected this and added some more test coverage.

rjmccall added inline comments.Dec 17 2020, 10:38 AM
clang/lib/Sema/SemaStmt.cpp
1848

You can have multiple tag declarations because of complex declarators or type-specifiers, e.g. struct A (*var)(struct B) or _Atomic(struct A(*)(struct B)). It should already be an error to write the latter without a declarator, but you do need to not diagnose if there *is* a declarator. Probably the right way to write this is to ignore tags in the first walk but remember whether you saw a non-tag; if you don't, diagnose the first tag.

Updating based on review feedback.

clang/lib/Sema/SemaStmt.cpp
1848

Good point on the more complex declarators -- I've added some extra tests for that case as well.

This revision is now accepted and ready to land.Dec 17 2020, 2:36 PM
aaron.ballman closed this revision.Dec 18 2020, 4:56 AM

Thank you for the review! I've commit in 2d2498ec6c42b12eae873257e6ddce3333fe8348