Skip to content

Commit

Permalink
[Feature] Better type hint error message
Browse files Browse the repository at this point in the history
Summary:
When user forget to turn on runtime option EnableHipHopSyntax, the type hint
error message can be confusing. For example,
[] cat /tmp/t.php
<?php
function foo(int $p = 1) {}
[] hphpi/hphpi /tmp/t.php
HipHop Fatal error: Default value with a class type hint can only be NULL in
/tmp/t.php on line 2

I made some changes to make the type hint error message more friendly. So after
the change:
[] hphpi/hphpi /tmp/t.php
HipHop Fatal error: HipHop type hint int is not enabled in /tmp/t.php on line 2

Test Plan:
make fast_tests

Reviewed By: qigao
Reviewers: qigao, mwilliams
CC: hphp-diffs@lists, ps, mwilliams, qigao
Revert Plan:
Tags:

- begin *PUBLIC* platform impact section -
Bugzilla: #
- end platform impact -

Differential Revision: 268072
  • Loading branch information
myang authored and macvicar committed Jun 16, 2011
1 parent 2d01b6f commit 13f43a3
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 33 deletions.
48 changes: 37 additions & 11 deletions src/runtime/eval/ast/construct.cpp
Expand Up @@ -23,24 +23,50 @@ using namespace std;
///////////////////////////////////////////////////////////////////////////////

Construct::TypePtrMap Construct::TypeHintTypes;
Construct::TypePtrMap Construct::HipHopTypeHintTypes;
Construct::TypePtrMap Construct::HipHopExperimentalTypeHintTypes;

const Construct::TypePtrMap &Construct::GetHipHopTypeHintTypes() {
if (HipHopTypeHintTypes.empty()) {
HipHopTypeHintTypes["bool"] = KindOfBoolean;
HipHopTypeHintTypes["boolean"] = KindOfBoolean;
HipHopTypeHintTypes["int"] = KindOfInt64;
HipHopTypeHintTypes["integer"] = KindOfInt64;
HipHopTypeHintTypes["real"] = KindOfDouble;
HipHopTypeHintTypes["double"] = KindOfDouble;
HipHopTypeHintTypes["float"] = KindOfDouble;
HipHopTypeHintTypes["string"] = KindOfString;
}
return HipHopTypeHintTypes;
}

const Construct::TypePtrMap &Construct::GetHipHopExperimentalTypeHintTypes() {
if (HipHopExperimentalTypeHintTypes.empty()) {
HipHopExperimentalTypeHintTypes["vector"] = KindOfArray;
HipHopExperimentalTypeHintTypes["map"] = KindOfArray;
HipHopExperimentalTypeHintTypes["set"] = KindOfArray;
}
return HipHopExperimentalTypeHintTypes;
}

const Construct::TypePtrMap &Construct::GetTypeHintTypes() {
if (TypeHintTypes.empty()) {
TypeHintTypes["array"] = KindOfArray;
if (RuntimeOption::EnableHipHopExperimentalSyntax) {
TypeHintTypes["vector"] = KindOfArray;
TypeHintTypes["map"] = KindOfArray;
TypeHintTypes["set"] = KindOfArray;
GetHipHopExperimentalTypeHintTypes();
for (Construct::TypePtrMap::const_iterator iter =
HipHopExperimentalTypeHintTypes.begin();
iter != HipHopExperimentalTypeHintTypes.end(); ++iter) {
TypeHintTypes[iter->first] = iter->second;
}
}
if (RuntimeOption::EnableHipHopSyntax) {
TypeHintTypes["bool"] = KindOfBoolean;
TypeHintTypes["boolean"] = KindOfBoolean;
TypeHintTypes["int"] = KindOfInt64;
TypeHintTypes["integer"] = KindOfInt64;
TypeHintTypes["real"] = KindOfDouble;
TypeHintTypes["double"] = KindOfDouble;
TypeHintTypes["float"] = KindOfDouble;
TypeHintTypes["string"] = KindOfString;
GetHipHopTypeHintTypes();
for (Construct::TypePtrMap::const_iterator iter =
HipHopTypeHintTypes.begin();
iter != HipHopTypeHintTypes.end(); ++iter) {
TypeHintTypes[iter->first] = iter->second;
}
}
}
return TypeHintTypes;
Expand Down
4 changes: 4 additions & 0 deletions src/runtime/eval/ast/construct.h
Expand Up @@ -37,6 +37,8 @@ class Construct {
public:
typedef hphp_string_imap<DataType> TypePtrMap;
static const TypePtrMap &GetTypeHintTypes();
static const TypePtrMap &GetHipHopTypeHintTypes();
static const TypePtrMap &GetHipHopExperimentalTypeHintTypes();

public:
Construct(CONSTRUCT_ARGS);
Expand Down Expand Up @@ -98,6 +100,8 @@ class Construct {
int _count;

static TypePtrMap TypeHintTypes;
static TypePtrMap HipHopTypeHintTypes;
static TypePtrMap HipHopExperimentalTypeHintTypes;
};

///////////////////////////////////////////////////////////////////////////////
Expand Down
69 changes: 47 additions & 22 deletions src/runtime/eval/ast/function_statement.cpp
Expand Up @@ -98,17 +98,7 @@ Parameter::Parameter(CONSTRUCT_ARGS, const string &type,
m_nullDefault = (dtype == KindOfNull &&
m_defVal->unsafe_cast<ScalarExpression>());
if (!checkTypeHint(m_kind, dtype)) {
if (m_kind == KindOfArray) {
parser->error("Default value with array type hint can only be "
"an array or NULL");
} else if (m_kind == KindOfObject) {
parser->error("Default value with a class type hint can only be "
"NULL");
} else {
ASSERT(RuntimeOption::EnableHipHopSyntax);
parser->error("Default value need to have the same type as "
"the type hint");
}
reportTypeHintError(NULL, type);
} else if (m_defVal->unsafe_cast<ScalarExpression>() ||
m_defVal->unsafe_cast<ArrayExpression>()) {
m_correct = true;
Expand Down Expand Up @@ -149,17 +139,7 @@ void Parameter::bindDefault(VariableEnvironment &env) const {
DataType dtype = v.getType();
ASSERT(dtype != KindOfUninit);
if (!checkTypeHint(m_kind, dtype)) {
if (m_kind == KindOfArray) {
raise_error("Default value with array type hint can only be "
"an array or NULL");
} else if (m_kind == KindOfObject) {
raise_error("Default value with a class type hint can only be "
"NULL");
} else {
ASSERT(RuntimeOption::EnableHipHopSyntax);
raise_error("Default value need to have the same type as "
"the type hint");
}
reportTypeHintError(NULL, m_type);
}
}
env.getIdx(m_idx)->assignVal(v);
Expand Down Expand Up @@ -665,6 +645,51 @@ Variant FunctionStatement::InvokerFewArgs(void *extra, int count,
collect_few_args_ref(count, INVOKE_FEW_ARGS_PASS_ARGS));
}

void Parameter::error(Parser *parser, const char *fmt, ...) const {
std::string msg;
va_list ap;
va_start(ap, fmt);
Util::string_vsnprintf(msg, fmt, ap);
va_end(ap);
if (parser) {
parser->error(msg);
} else {
raise_error(msg);
}
}

void Parameter::reportTypeHintError(Parser *parser, const string &hintType)
const {
if (m_kind == KindOfArray) {
error(parser,
"Default value with array type hint can only be an array or NULL");
} else if (m_kind == KindOfObject) {
bool isHipHopTypeHint = false;
bool isHipHopExperimentalTypeHint = false;
if (GetHipHopTypeHintTypes().find(hintType) !=
GetHipHopTypeHintTypes().end()) {
isHipHopTypeHint = true;
} else if (GetHipHopExperimentalTypeHintTypes().find(hintType) !=
GetHipHopExperimentalTypeHintTypes().end()) {
isHipHopExperimentalTypeHint = true;
}
if (!RuntimeOption::EnableHipHopSyntax && isHipHopTypeHint) {
error(parser,
"HipHop type hint %s is not enabled", hintType.c_str());
} else if (!RuntimeOption::EnableHipHopExperimentalSyntax &&
isHipHopExperimentalTypeHint) {
error(parser, "HipHop experimental type hint %s is not enabled",
hintType.c_str());
} else {
error(parser, "Default value with a class type hint can only be NULL");
}
} else {
ASSERT(RuntimeOption::EnableHipHopSyntax ||
RuntimeOption::EnableHipHopExperimentalSyntax);
error(parser, "Default value need to have the same type as the type hint");
}
}

///////////////////////////////////////////////////////////////////////////////
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/runtime/eval/ast/function_statement.h
Expand Up @@ -71,6 +71,8 @@ class Parameter : public Construct {
bool m_correct;
private:
bool checkTypeHint(DataType hint, DataType type) const;
void error(Parser *parser, const char *fmt, ...) const;
void reportTypeHintError(Parser *parser, const std::string &hintType) const;
};

class FunctionStatement : public Statement, public Block, public Function {
Expand Down
4 changes: 4 additions & 0 deletions src/runtime/eval/parser/parser.cpp
Expand Up @@ -250,6 +250,10 @@ void Parser::error(const char* fmt, ...) {
raise_error("%s", msg.c_str());
}

void Parser::error(const string &msg) {
error(msg.c_str());
}

void Parser::warning(const char* fmt, ...) {
va_list ap;
va_start(ap, fmt);
Expand Down
1 change: 1 addition & 0 deletions src/runtime/eval/parser/parser.h
Expand Up @@ -171,6 +171,7 @@ class Parser : public ParserBase {
// implementing ParserBase
virtual bool parse();
virtual void error(const char* fmt, ...);
virtual void error(const std::string &msg);
virtual void warning(const char* fmt, ...);
virtual void warning(const std::string &msg);
virtual bool enableXHP();
Expand Down

0 comments on commit 13f43a3

Please sign in to comment.