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

Reply via email to