[Fix] declareConstant should return true on success

Summary:
declareConstant returned false for illegal values, but the defined value for
everything else. It should return true for success, and false otherwise.

Also add warning for the redeclared case, since it should be rare.

Test Plan: fast_tests slow_tests

Reviewers: myang, qigao

Reviewed By: myang

CC: ps, mwilliams, myang

Differential Revision: 345637

src/runtime/base/hphp_system.cpp

@@ -92,17 +92,19 @@ void Globals::initialize() {
   }
 }
 
-CVarRef Globals::declareConstant(CStrRef name, Variant &constant,
-                                 CVarRef value) {
+bool Globals::declareConstant(CStrRef name, Variant &constant,
+                              CVarRef value) {
   if (!value.isAllowedAsConstantValue()) {
     raise_warning("Constants may only evaluate to scalar values");
-    return false_varNR;
+    return false;
   }
   if (!m_dynamicConstants.exists(name)) {
     m_dynamicConstants.set(name, value);
     constant = value;
+    return true;
   }
-  return value;
+  raise_warning("Constant %s already defined", name.data());
+  return false;
 }
 
 void Globals::declareFunction(const char *name) {

src/runtime/base/hphp_system.h

@@ -53,7 +53,7 @@ class Globals : public LVariableTable {
 public:
   Globals() : FVF(__autoload)(false) {}
   void initialize();
-  CVarRef declareConstant(CStrRef name, Variant &constant, CVarRef value);
+  bool declareConstant(CStrRef name, Variant &constant, CVarRef value);
   void declareFunction(const char *name);
   void declareFunctionLit(CStrRef name);
   bool defined(CStrRef name);

src/test/test_code_run.cpp

@@ -14629,6 +14629,10 @@ bool TestCodeRun::TestConstant() {
        "var_dump(BLAH);"
        "define('FOO', array(1,2,3));");
 
+  MVCR("<?php "
+       "var_dump(define('foo', false));"
+       "var_dump(define('foo', true));");
+
   return true;
 }