Mordante marked 2 inline comments as done. Mordante added inline comments.
================ Comment at: clang/lib/Sema/SemaStmt.cpp:578 +static std::pair<Stmt::Likelihood, const Attr *> +getLikelihood(const Stmt *Stmt) { + if (const auto *AS = dyn_cast<AttributedStmt>(Stmt)) ---------------- rsmith wrote: > Mordante wrote: > > aaron.ballman wrote: > > > rsmith wrote: > > > > Sema doesn't care about any of this; can you move this code to CodeGen > > > > and remove the code that stores likelihood data in the AST? > > > FWIW, I asked for it to be moved out of CodeGen and into Sema because the > > > original implementation in CodeGen was doing a fair amount of work trying > > > to interrogate the AST for this information. Now that we've switched the > > > design to only be on the substatement of an if/else statement (rather > > > than an arbitrary statement), it may be that CodeGen is a better place > > > for this again (and if so, I'm sorry for the churn). > > At the moment the Sema cares about it to generate diagnostics about > > conflicting annotations: > > ``` > > if(i) [[ likely]] {} > > else [[likely]] {} > > ``` > > This diagnostic only happens for an if statement, for a switch multiple > > values can be considered likely. > > Do you prefer to have the diagnostic also in the CodeGen? > It looked to me like you'd reached agreement to remove that diagnostic. Are > you intending to keep it? > > If so, then I'd suggest you make `getLikelihood` a member of `Stmt` so that > `CodeGen` and the warning here can both easily call it. @aaron.ballman I thought we wanted to keep this conflict warning, am I correct? I'll add an extra function to the Stmt to test for a conflict and use that for the diagnostic in the Sema. This allows me to use `LH_None` for no attribute or a conflict of attributes in the CodeGen. Then there's no need for `LH_Conflict` and it can be removed from the enumerate. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85091/new/ https://reviews.llvm.org/D85091 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits