[PATCH] D142388: [clang] Add builtin_nondet

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/Builtins.def:658
 BUILTIN(__builtin_call_with_static_chain, "v.", "nt")
+BUILTIN(__builtin_nondet, "v.", "nt")
 
----------------
ManuelJBrito wrote:
> ManuelJBrito wrote:
> > ManuelJBrito wrote:
> > > erichkeane wrote:
> > > > aaron.ballman wrote:
> > > > > aaron.ballman wrote:
> > > > > > shafik wrote:
> > > > > > > erichkeane wrote:
> > > > > > > > Not a fan of the name in general, it doesn't really explain what is happening.  Perhaps `__builtin_nondeterministic_value` or something?  
> > > > > > > > 
> > > > > > > > I also wonder if others might have a better name :) 
> > > > > > > `__builtin_indeterminate_value` wdyt?
> > > > > > I think `__builtin_unspecified_value()` makes sense, as that's what you're getting back (there is a valid value, but you may get a different value on every call). I don't think `indeterminate` makes sense because that implies it may return trap values, and I'm guessing that's not behavior we want here (or am I wrong about that)?
> > > > > > 
> > > > > > Also, should this be usable within a constant expression? I would assume not; we should have a test case showing you can't use it in such a context.
> > > > > Why does this require custom typechecking?
> > > > Our issue last time with "unspecified" value was that this has a distinct meaning in C/C++, so we don't want to use that word.  
> > > > I don't think `indeterminate` makes sense because that implies it may return trap values, and I'm guessing that's not behavior we want here (or am I wrong about that)?
> > > 
> > > The idea is for it to return some correct value of the same type as the argument, so no trap values.
> > > 
> > > Can we settle on `__builtin_nondeterministic_value`? 
> > > Why does this require custom typechecking?
> > 
> > I wasn't sure how to have the builtin take one argument of any type. 
> > Also, should this be usable within a constant expression? I would assume not; we should have a test case showing you can't use it in such a context.
> 
> Can you shed a light on the implications of this? Or point me to some reference?
> 
> Can we settle on `__builtin_nondeterministic_value`?

I can live with it.

> Our issue last time with "unspecified" value was that this has a distinct meaning in C/C++, so we don't want to use that word.

I guess I don't see the issue; the behavior of the builtin as I understand it matches that distinct meaning in C/C++ (and English prose).

>>Also, should this be usable within a constant expression? I would assume not; we should have a test case showing you can't use it in such a context.
> Can you shed a light on the implications of this? Or point me to some reference?

It boils down to whether you think this should be valid:
```
constexpr bool hoo_boy() {
  return __builtin_nondeterministic_value() == 42;
}
constexpr bool val = hoo_boy();
```
(among other uses of constant expressions). My intuition is that there's nothing useful you could do with this builtin in a constant expression (my example is generally always going to be false except for the times when it's true, lol), but if all we're doing is giving you *a* value, nothing says we can't generate that value at compile time so it seems possible to support.

>>Why does this require custom typechecking?
> I wasn't sure how to have the builtin take one argument of any type.

I was thinking it can use `.` to specify that the function is varargs, and then SemaChecking.cpp can validate that only one argument is passed to the call, but now that I think about that.... maybe we don't want that behavior because that would induce default argument promotions which would change the type (for example, from float to double). So on reflection, I think this is fine as-is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142388