r310905 - Avoid PointerIntPair of constexpr EvalInfo structs

[David Blaikie via cfe-commits]

Feels like this alignment attribute may be a bit of a death trap, then?
Should other uses of the attribute be scrutinized, or could we come up with
a portable way to use it safely? (using a different attribute on different
compilers - sounded like maybe one form worked with MSVC and a different
form worked with GCC? Maybe the attribute could be improved to use those as
appropriate?)

On Mon, Aug 14, 2017 at 6:18 PM Reid Kleckner via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: rnk
> Date: Mon Aug 14 18:17:47 2017
> New Revision: 310905
>
> URL: http://llvm.org/viewvc/llvm-project?rev=310905&view=rev
> Log:
> Avoid PointerIntPair of constexpr EvalInfo structs
>
> They are stack allocated, so their alignment is not to be trusted.
> 32-bit MSVC only guarantees 4 byte stack alignment, even though alignof
> would tell you otherwise. I tried fixing this with __declspec align, but
> that apparently upsets GCC. Hopefully this version will satisfy all
> compilers.
>
> See PR32018 for some info about the mingw issues.
>
> Should supercede https://reviews.llvm.org/D34873
>
> Modified:
>     cfe/trunk/lib/AST/ExprConstant.cpp
>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=310905&r1=310904&r2=310905&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Aug 14 18:17:47 2017
> @@ -537,7 +537,7 @@ namespace {
>    /// rules.  For example, the RHS of (0 && foo()) is not evaluated.  We
> can
>    /// evaluate the expression regardless of what the RHS is, but C only
> allows
>    /// certain things in certain situations.
> -  struct LLVM_ALIGNAS(/*alignof(uint64_t)*/ 8) EvalInfo {
> +  struct EvalInfo {
>      ASTContext &Ctx;
>
>      /// EvalStatus - Contains information about the evaluation.
> @@ -977,24 +977,22 @@ namespace {
>    /// RAII object used to optionally suppress diagnostics and
> side-effects from
>    /// a speculative evaluation.
>    class SpeculativeEvaluationRAII {
> -    /// Pair of EvalInfo, and a bit that stores whether or not we were
> -    /// speculatively evaluating when we created this RAII.
> -    llvm::PointerIntPair<EvalInfo *, 1, bool> InfoAndOldSpecEval;
> -    Expr::EvalStatus Old;
> +    EvalInfo *Info;
> +    Expr::EvalStatus OldStatus;
> +    bool OldIsSpeculativelyEvaluating;
>
>      void moveFromAndCancel(SpeculativeEvaluationRAII &&Other) {
> -      InfoAndOldSpecEval = Other.InfoAndOldSpecEval;
> -      Old = Other.Old;
> -      Other.InfoAndOldSpecEval.setPointer(nullptr);
> +      Info = Other.Info;
> +      OldStatus = Other.OldStatus;
> +      Other.Info = nullptr;
>      }
>
>      void maybeRestoreState() {
> -      EvalInfo *Info = InfoAndOldSpecEval.getPointer();
>        if (!Info)
>          return;
>
> -      Info->EvalStatus = Old;
> -      Info->IsSpeculativelyEvaluating = InfoAndOldSpecEval.getInt();
> +      Info->EvalStatus = OldStatus;
> +      Info->IsSpeculativelyEvaluating = OldIsSpeculativelyEvaluating;
>      }
>
>    public:
> @@ -1002,8 +1000,8 @@ namespace {
>
>      SpeculativeEvaluationRAII(
>          EvalInfo &Info, SmallVectorImpl<PartialDiagnosticAt> *NewDiag =
> nullptr)
> -        : InfoAndOldSpecEval(&Info, Info.IsSpeculativelyEvaluating),
> -          Old(Info.EvalStatus) {
> +        : Info(&Info), OldStatus(Info.EvalStatus),
> +          OldIsSpeculativelyEvaluating(Info.IsSpeculativelyEvaluating) {
>        Info.EvalStatus.Diag = NewDiag;
>        Info.IsSpeculativelyEvaluating = true;
>      }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170829/da66ca10/attachment.html>