[PATCH] D60386: FileCheck [6/12]: Introduce numeric variable definition

[Paul Robinson via Phabricator via llvm-commits]

probinson added inline comments.


================
Comment at: llvm/include/llvm/Support/FileCheck.h:355
 public:
   explicit FileCheckPattern(Check::FileCheckType Ty,
+                            FileCheckPatternContext *Context, unsigned Line)
----------------
jhenderson wrote:
> probinson wrote:
> > jhenderson wrote:
> > > probinson wrote:
> > > > Multiple parameters means you don't need 'explicit'.
> > > There's still an explicit here (but it's worth noting that `explicit` does still have an effect on multi-parameter constructor).
> > Really? What effect does `explicit` have on a multi-parameter constructor? My reading of the cppreference.org description is that it used to apply to `T var = {args};` but that is list-initialization starting with C++11.
> The example on https://en.cppreference.com/w/cpp/language/explicit, shows that copy-list-initialization does not consider conversion constructors marked with `explicit`, from C++11. Take a look at the A3, A4, B3 and B4 examples. Summary version is that `A var = {arg1, arg2};` will match a constructor without explicit, but not one with, whilst `A var {arg1, arg2};` will work for both.
> 
> Mind you, it's not likely to actually matter in most instances, and we may not actually want the behaviour of `explicit`.
Okay, C++ is just too weird.
My point, and I did have one, is that `explicit` on a multi-parameter ctor is generally not used within LLVM, because the syntax where it matters is obviously construction or initialization anyway.  LLVM tends to prefer the less-verbose syntax where there's no real benefit to the more-verbose syntax. The `explicit` was appropriate when this was a one-parameter ctor, and isn't really useful now, so it should be removed.
Or, if @thopre thinks it is useful, say why.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60386