mibintc marked an inline comment as done.
mibintc added inline comments.
================
Comment at: clang/include/clang/AST/Expr.h:3465
+ SubExprs[RHS] = rhs;
+ hasFPFeatures = true;
+ isCompoundAssign = 1;
----------------
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.
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
