Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HPCC-20590 Suppress all semantic errors when syntax checking #11735

Merged
merged 2 commits into from Oct 2, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion ecl/hql/hqlexpr.hpp
Expand Up @@ -1022,7 +1022,6 @@ class HQL_API HqlLookupContext
inline bool syntaxChecking() const { return parseCtx.isSyntaxChecking(); }
inline bool regenerateCache() const { return parseCtx.regenerateCache; }
inline bool hasCacheLocation() const { return parseCtx.hasCacheLocation();}
inline bool syntaxCheckingUsingCache() const { return parseCtx.isSyntaxChecking() && parseCtx.hasCacheLocation(); }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed this function because after commoning up the code below this function is used just once.

inline bool checkSimpleDef() const { return parseCtx.checkSimpleDef; }
inline bool ignoreCache() const { return parseCtx.ignoreCache; }
inline bool createCache(IHqlExpression * simplified, bool isMacro) { return parseCtx.createCache(simplified, isMacro); }
Expand Down
25 changes: 19 additions & 6 deletions ecl/hql/hqlgram2.cpp
Expand Up @@ -6169,9 +6169,22 @@ void HqlGram::reportErrorUnexpectedX(const attribute& errpos, IAtom * unexpected
reportError(ERR_UNEXPECTED_ATTRX, errpos, "Unexpected attribute %s", str(unexpected));
}

static bool includeError(HqlLookupContext & ctx, WarnErrorCategory category)
{
if (ctx.syntaxChecking() && !ctx.ignoreCache())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the PR is ok, but this is the line I am debating about. And my debate is more about the corresponding call in hqlgram2.cpp(12353)

I am trying to work out what options can be used to regression test the private suite. I think it requires an empty --metacache, syntax checklng and ignore cache to be not set.

{
if (category==CategorySyntax && category==CategoryError)
return true;
else
return false;
}
else
return true;
}

void HqlGram::doReportWarning(WarnErrorCategory category, int warnNo, const char *msg, const char *filename, int lineno, int column, int pos)
{
if (!lookupCtx.syntaxCheckingUsingCache() || category==CategorySyntax || category==CategoryError)
if (includeError(lookupCtx, category))
{
Owned<IError> error = createError(category, queryDefaultSeverity(category), warnNo, msg, filename, lineno, column, pos);
report(error);
Expand Down Expand Up @@ -6216,7 +6229,7 @@ void HqlGram::reportError(int errNo, const char *msg, int lineno, int column, in

void HqlGram::reportWarning(WarnErrorCategory category, int warnNo, const ECLlocation & pos, const char* format, ...)
{
if (errorHandler && !errorDisabled && (!lookupCtx.syntaxCheckingUsingCache() || category==CategorySyntax || category==CategoryError))
if (errorHandler && !errorDisabled && includeError(lookupCtx, category))
{
va_list args;
va_start(args, format);
Expand All @@ -6229,7 +6242,7 @@ void HqlGram::reportWarning(WarnErrorCategory category, int warnNo, const ECLloc

void HqlGram::reportWarning(WarnErrorCategory category, ErrorSeverity severity, int warnNo, const ECLlocation & pos, const char* format, ...)
{
if (errorHandler && !errorDisabled && (!lookupCtx.syntaxCheckingUsingCache() || category==CategorySyntax || category==CategoryError))
if (errorHandler && !errorDisabled && includeError(lookupCtx, category))
{
StringBuffer msg;
va_list args;
Expand All @@ -6244,7 +6257,7 @@ void HqlGram::reportWarning(WarnErrorCategory category, ErrorSeverity severity,
void HqlGram::reportWarningVa(WarnErrorCategory category, int warnNo, const attribute& a, const char* format, va_list args)
{
const ECLlocation & pos = a.pos;
if (errorHandler && !errorDisabled && (!lookupCtx.syntaxCheckingUsingCache() || category==CategorySyntax || category==CategoryError))
if (errorHandler && !errorDisabled && includeError(lookupCtx, category))
{
Owned<IError> error = createErrorVA(category, queryDefaultSeverity(category), warnNo, pos, format, args);
report(error);
Expand All @@ -6253,7 +6266,7 @@ void HqlGram::reportWarningVa(WarnErrorCategory category, int warnNo, const attr

void HqlGram::reportWarning(WarnErrorCategory category, int warnNo, const char *msg, int lineno, int column)
{
if (errorHandler && !errorDisabled && (!lookupCtx.syntaxCheckingUsingCache() || category==CategorySyntax || category==CategoryError))
if (errorHandler && !errorDisabled && includeError(lookupCtx, category))
doReportWarning(category, warnNo, msg, querySourcePathText(), lineno, column, 0);
}

Expand Down Expand Up @@ -12280,7 +12293,7 @@ void parseAttribute(IHqlScope * scope, IFileContents * contents, HqlLookupContex
attrCtx.noteBeginAttribute(scope, contents, name);

// Use simplified definition if syntax checking
if (!ctx.regenerateCache() && ctx.syntaxCheckingUsingCache() && !ctx.ignoreCache())
if (!ctx.regenerateCache() && ctx.syntaxChecking() && ctx.hasCacheLocation() && !ctx.ignoreCache())
{
HqlParseContext & parseContext = ctx.queryParseContext();
Owned<IEclCachedDefinition> cached = parseContext.cache->getDefinition(fullName);
Expand Down