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

Reply via email to