erichkeane added inline comments.
================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1127
return false;
- if (!this->emitInitGlobal(*T, *I, VD))
+ }
+ } else {
----------------
aaron.ballman wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > tbaeder wrote:
> > > > aaron.ballman wrote:
> > > > > and if we have no `GlobalIndex`?
> > > > >
> > > > > Should this instead be:
> > > > > ```
> > > > > if (auto GlobalIndex = P.getGlobal(VD); !GlobalIndex ||
> > > > > !this->emitGetPtrGlobal(*GlobalIndex, VD))
> > > > > return false;
> > > > > ```
> > > > >
> > > > I mean, that shouldn't happen because the `visitVarDecl` call before
> > > > would've returned `false` in that case.
> > > The use of an `if` makes it seem like it could fail. Perhaps we should
> > > drop the `if` and use an `assert`?
> > What do you think about returning a `tuple(valid, offset)` from
> > `visitVarDecl`? Although that might make the call in `ByteCodeStmtGen.cpp`
> > a little awkward.
> Hmmmm, I don't think I'd like to see a tuple, but seeing an `llvm::ErrorOr<>`
> or a `std::optional<>` might be reasonable. The downside is that the `visit*`
> functions then won't have a uniform return type (can't do `ErrorOr<void>`
> that I'm aware of)
The way I've seen in the past is to have it be an error-type containing a
nullptr_t (or some sort of empty/meaningless struct).
So something like:
`ErrorOr<Void>`
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136815/new/
https://reviews.llvm.org/D136815
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
