rZhBoYao marked 2 inline comments as done.
rZhBoYao added inline comments.

================
Comment at: clang/include/clang/Sema/Sema.h:2899-2909
+    /// C++ [dcl.fct.def.general]p1
+    /// function-body:
+    ///   = delete ;
+    ///   = default ;
+    Delete,
+    Default,
+
----------------
ChuanqiXu wrote:
> rZhBoYao wrote:
> > ChuanqiXu wrote:
> > > Agree to @erichkeane 
> > With all due respect, this code suggestion doesn't make any sense to me. My 
> > best guess is @ChuanqiXu was thinking the order specified by the grammar as 
> > noted in [[ https://eel.is/c++draft/dcl.fct.def.general#nt:function-body | 
> > dcl.fct.def.general p1 ]]. Even if that was the case, `CompoundStmt` is not 
> > quite right either. Also, differentiating `ctor-initializer[opt] 
> > compound-statement` and `function-try-block` is meaningless here, hence the 
> > name `Other`.
> > 
> > I adopted the same order as to how `Parser::ParseFunctionDefinition` has 
> > always been parsing `function-body`. The order is not significant in any 
> > meaningful way as each of the 4 grammar productions of `function-body` is 
> > VERY different and mutually exclusive. Putting `Delete` and `Default` 
> > upfront not only emphasizes the "specialness" of them but also conveys how 
> > we handle `function-body`.
> > 
> > What say you, @erichkeane ?
> Yeah, the order comes from the standard. I think the comment should be 
> consistent with the spec. And for the name, I agree `CompoundStmt` is not 
> accurate indeed... I thought Default should be a good name but there is 
> `Default` already. But I don't feel `Other` is good since the 
> compound-statement is much more common.
> 
> > Putting Delete and Default upfront not only emphasizes the "specialness" of 
> > them but also conveys how we handle function-body.
> 
> This don't makes sense. Generally, we would put common items in front. And 
> the order here shouldn't be related to the implementation.
> But I don't feel `Other` is good since the compound-statement is much more 
> common.
`Other` reads "not special like `Delete` and `Default`".
> This don't makes sense. Generally, we would put common items in front. And 
> the order here shouldn't be related to the implementation.
Think of it as a not-so-special else clause at the end of an if/else chain. We 
tend to process special cases upfront. It is only natural.

I'll await @erichkeane 's final verdict.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122981/new/

https://reviews.llvm.org/D122981

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to