ehsan added a comment.

In http://reviews.llvm.org/D16465#333688, @rnk wrote:

> Your code won't catch this test case:
>
>   static int x;
>   extern inline const bool *f() {
>     static const bool p = !&x;
>     return &p;
>   }
>
> Getting this exactly right is going to be a challenge. =/


Oh you're right.  I need to spend some more time comparing our behavior with 
cl's, it seems...


================
Comment at: include/clang/Basic/DiagnosticASTKinds.td:151
@@ -150,1 +150,3 @@
   "%plural{1:byte|:bytes}1">;
+def note_constexpr_microsoft_abi_declrefexpr : Note<
+  "the constant expression cannot contain a reference to a variable as a 
Microsoft "
----------------
rnk wrote:
> We should add this test case and decide what to do with it:
>   static int x;
>   inline int **f() {
>     static constexpr int *p = true ? 0 : &x;
>     return &p;
>   }
> Currently, in your patch, this diagnostic will come out. MSVC compiles this 
> to guarded, dynamic initialization, despite the constexpr. ;_;
> 
> David thinks we should just give the user the real deal constexpr behavior, 
> even though it's ABI incompatible.
One solution to this would be to create a variation of `evaluateValue()` which 
activates this new behavior and only use it from 
`CodeGenModule::EmitConstantInit()`, so that the behavior of `evaluateValue()` 
in this context doesn't change.  How does that sound?

By the way, I just realized that 
`CheckPotentialExpressionContainingDeclRefExpr()` eats this diagnostic because 
of the `SpeculativeLookForDeclRefExprRAII` object.  Should I propagate it up 
from `CheckPotentialExpressionContainingDeclRefExpr()` to make it user visible? 
 Right now the test case above only emits "constexpr variable 'p' must be 
initialized by a constant expression" without any notes.

================
Comment at: lib/AST/ExprConstant.cpp:9008
@@ -8917,1 +9007,3 @@
 
+  if (Ctx.getTargetInfo().getCXXABI().isMicrosoft())
+    InitInfo.useMicrosoftABI();
----------------
rnk wrote:
> This should be limited in scope to only apply to static locals. We should be 
> able to statically initialize globals.
Right, my bad!


http://reviews.llvm.org/D16465



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to