rjmccall added inline comments.
================
Comment at: clang/include/clang/AST/Expr.h:3465
+ SubExprs[RHS] = rhs;
+ hasFPFeatures = true;
+ isCompoundAssign = 1;
----------------
mibintc wrote:
> rjmccall wrote:
> > Okay, so this is *always* adding trailing storage. Is there a common value
> > for `FPOptions` — something that corresponds to default settings, for
> > example — that we could use to avoid needing storage in the common case?
> >
> > Also, it would be useful to extract a function somewhere that you can use
> > in all of these places for consistency, maybe something like this on
> > `FPOptions`:
> >
> > ```
> > /// Does this FPOptions require trailing storage when stored in various
> > AST nodes,
> > /// or can it be recreated using `defaultWithoutTrailingStorage`?
> > bool requiresTrailingStorage() const { return *this ==
> > defaultWithoutTrailingStorage(); }
> >
> > /// Return the default value of FPOptions that's used when trailing
> > storage isn't required.
> > static FPOptions defaultWithoutTrailingStorage() { ... }
> > ```
> The reason I set hasFPFeatures to True in this revision is because in the
> previous review you asked me to make it purely a representational change and
> "add stuff about whether there is a pragma in a separate patch". The stuff I
> had in the previous version was smarter about creating trailing storage, it
> only created trailing storage when the pragma was in effect. I envirsioned
> that the evolution would accept something that always created the FPOptions
> in trailing storage, and in the 2nd generation would adopt the more thrifty
> way of creating trailing storage.
Well, let's at least set up the infrastructure and APIs correctly, even if we
always allocate trailing storage.
For example, should `getFPOptions` take an `ASTContext &` so that it can always
return the right options instead of making clients do `hasFPOptions() ?
getFPOptions() : ctx.getFPOptions()`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76384/new/
https://reviews.llvm.org/D76384
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
