haberman added inline comments.
================ Comment at: clang/lib/Sema/SemaStmt.cpp:561-568 + for (const auto *A : Attrs) { + if (A->getKind() == attr::MustTail) { + if (!checkMustTailAttr(SubStmt, *A)) { + return SubStmt; + } + setFunctionHasMustTail(); + } ---------------- haberman wrote: > aaron.ballman wrote: > > haberman wrote: > > > aaron.ballman wrote: > > > > This functionality belongs in SemaStmtAttr.cpp, I think. > > > That is where I had originally put it, but that didn't work for > > > templates. The semantic checks can only be performed at instantiation > > > time. `ActOnAttributedStmt` seems to be the right hook point where I can > > > evaluate the semantic checks for both template and non-template functions > > > (with template functions getting checked at instantiation time). > > I disagree that `ActOnAttributedStmt()` is the correct place for this > > checking -- template checking should occur when the template is > > instantiated, same as happens for declaration attributes. I'd like to see > > this functionality moved to SemaStmtAttr.cpp. Keeping the attribute logic > > together and following the same patterns is what allows us to tablegenerate > > more of the attribute logic. Statement attributes are just starting to get > > more such automation. > I tried commenting out this code and adding the following code into > `handleMustTailAttr()` in `SemaStmtAttr.cpp`: > > ``` > if (!S.checkMustTailAttr(St, MTA)) > return nullptr; > ``` > > This caused my test cases related to templates to fail. It also seemed to > break test cases related to `JumpDiagnostics`. My interpretation of this is > that `handleMustTailAttr()` is called during parsing only, and cannot catch > errors at template instantiation time or that require a more complete AST. > > What am I missing? Where in SemaStmtAttr.cpp are you suggesting that I put > this check? Scratch the part about `JumpDiagnostics`, that was me failing to call `S.setFunctionHasMustTail()`. I added that and now the `JumpDiagnostics` tests pass. But the template test cases still fail, and I can't find any hook point in `SemaStmtAttr.cpp` that will let me evaluate these checks at template instantiation time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits