rZhBoYao added a comment. Revisiting @ChuanqiXu 's code suggestion...
================ Comment at: clang/include/clang/Sema/Sema.h:2899-2909 + /// C++ [dcl.fct.def.general]p1 + /// function-body: + /// = delete ; + /// = default ; + Delete, + Default, + ---------------- erichkeane wrote: > rZhBoYao wrote: > > 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. > >>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. > > This was my understanding. This is a single value that means 'not default > and not deleted'. I think the idea of doing it in standards order is a > really good one, but have the top 2 just both result in `Other`, `Unknown`, > etc. So something like: > > ``` > //ctor-initializeropt compound-statement > //function-try-block > Unknown, > //= default ; > Default, > //= delete ; > Delete > ``` > > BUT as this is not an 'ast' level flag, it is just for the purposes of > simplifying this call, I don't think it needs to conform that closely. I actually don't know what you were trying to say here: > We don't yet part function-try-block yet. > FunctionTryBlock, especially: > We don't yet part function-try-block yet. Nonetheless, I did my best to cater to your comment within reason, such as reordering enumerators to align with the C++ standard text. Repository: rG LLVM Github Monorepo 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
