GitHub PullRequestReviewCommentEvent at 2014-03-11 09:14:52
Repository: HPCC-Platform
Actor: ghalliday
Payload: {"comment":{"url":"https://api.github.com/repos/hpcc-systems/HPCC-Platform/pulls/comments/10465787","id":10465787,"diff_hunk":"@@ -6450,162 +6470,225 @@ IAtom * getWarningAction(unsigned errorCode, const HqlExprArray & overrides, uns\n     {\n         IHqlExpression & cur = overrides.item(i);\n         if (matchesConstantValue(cur.queryChild(0), errorCode))\n-            return cur.queryChild(1)->queryName();\n+            return getCheckSeverity(cur.queryChild(1)->queryName());\n     }\n-    return defaultAtom;\n+    return defaultSeverity;\n }\n \n-WarningProcessor::WarningProcessor() \n-{ \n-    firstLocalOnWarning = 0; \n-    activeSymbol = NULL; \n+//---------------------------------------------------------------------------\n+\n+ErrorSeverityMapper::ErrorSeverityMapper(IErrorReceiver & _errorProcessor) : IndirectErrorReceiver(_errorProcessor)\n+{\n+    firstActiveMapping = 0;\n+    activeSymbol = NULL;\n }\n \n \n-void WarningProcessor::addWarning(IECLError * warning)\n+const char * severityText[] = {\n+\n+};\n+\n+bool ErrorSeverityMapper::addCommandLineMapping(const char * mapping)\n {\n-    //warnings are assumed to be infrequent, so don't worry about efficiency here.\n-    IAtom * action = getWarningAction(warning->errorCode(), localOnWarnings, firstLocalOnWarning);\n+    if (!mapping)\n+        return true;\n \n-    if (action == defaultAtom)\n-        appendUnique(possibleWarnings, warning);\n-    else if (action == warningAtom)\n-        appendUnique(warnings, warning);\n-    else if (action == errorAtom)\n+    unsigned len = strlen(mapping);\n+    if (len == 0)\n+        return true;\n+\n+    const char * equals = strchr(mapping, '=');\n+    const char * value;\n+    if (equals)\n+        value = equals+1;\n+    else if (mapping[len-1] == '+')\n     {\n-        allErrors.append(*changeErrorType(true, warning));\n+        len--;\n+        value = \"error\";\n+    }\n+    else if (mapping[len-1] == '-')\n+    {\n+        value = \"ignore\";\n+        len--;\n     }\n     else\n+        value = \"error\";\n+\n+    IAtom * action = createAtom(value);\n+    if (getSeverity(action) == SeverityUnknown)\n+    {\n+        fprintf(stderr, \"invalid warning severity '%s'\\n\", value);\n+        return false;\n+    }\n+\n+    if (isdigit(mapping[0]))\n     {\n-        assertex(action == ignoreAtom);\n+        unsigned errorCode = atoi(mapping);\n+        addOnWarning(errorCode, action);\n+        return true;\n+    }\n+    else\n+    {\n+        fprintf(stderr, \"mapping doesn't specify a valid warning code '%s'\\n\", mapping);\n+        return false;\n     }\n }\n \n-void WarningProcessor::addGlobalOnWarning(unsigned code, IAtom * action)\n+void ErrorSeverityMapper::addOnWarning(unsigned code, IAtom * action)\n {\n-    globalOnWarnings.append(*createAttribute(onWarningAtom, getSizetConstant(code), createAttribute(action)));\n+    severityMappings.append(*createAttribute(onWarningAtom, getSizetConstant(code), createAttribute(action)));\n }\n \n-void WarningProcessor::addGlobalOnWarning(IHqlExpression * setMetaExpr)\n+void ErrorSeverityMapper::addOnWarning(IHqlExpression * setMetaExpr)\n {\n-    globalOnWarnings.append(*createAttribute(onWarningAtom, LINK(setMetaExpr->queryChild(1)), LINK(setMetaExpr->queryChild(2))));\n+    severityMappings.append(*createAttribute(onWarningAtom, LINK(setMetaExpr->queryChild(1)), LINK(setMetaExpr->queryChild(2))));\n }\n \n-void WarningProcessor::processMetaAnnotation(IHqlExpression * expr)\n+unsigned ErrorSeverityMapper::processMetaAnnotation(IHqlExpression * expr)\n {\n-    gatherMetaAttributes(localOnWarnings, onWarningAtom, expr);\n+    unsigned prevMax = severityMappings.ordinality();\n+    gatherMetaAttributes(severityMappings, onWarningAtom, expr);\n+    return prevMax;\n }\n \n-void WarningProcessor::processWarningAnnotation(IHqlExpression * expr)\n+void ErrorSeverityMapper::restoreLocalOnWarnings(unsigned prevMax)\n {\n-    //would be cleaner if each annotation defined an interface, and this was a dynamic cast\n-    //but not sufficiently complicated to warrent it.\n-    IECLError * error = static_cast<CHqlWarningAnnotation *>(expr)->queryWarning();\n-    addWarning(error);\n+    severityMappings.trunc(prevMax);\n }\n \n-void WarningProcessor::pushSymbol(OnWarningState & saved, IHqlExpression * _symbol)\n+void ErrorSeverityMapper::pushSymbol(ErrorSeverityMapperState & saved, IHqlExpression * _symbol)\n {\n     saveState(saved);\n     setSymbol(_symbol);\n }\n \n-void WarningProcessor::saveState(OnWarningState & saved)\n+void ErrorSeverityMapper::saveState(ErrorSeverityMapperState & saved) const\n {\n-    saved.firstOnWarning = firstLocalOnWarning;\n-    saved.onWarningMax = localOnWarnings.ordinality();\n+    saved.firstActiveMapping = firstActiveMapping;\n+    saved.maxMappings = severityMappings.ordinality();\n     saved.symbol = activeSymbol;\n }\n \n-void WarningProcessor::setSymbol(IHqlExpression * _symbol)\n+void ErrorSeverityMapper::setSymbol(IHqlExpression * _symbol)\n {\n-    firstLocalOnWarning = localOnWarnings.ordinality();\n+    firstActiveMapping = severityMappings.ordinality();\n     activeSymbol = _symbol;\n }\n \n-void WarningProcessor::restoreState(const OnWarningState & saved)\n+void ErrorSeverityMapper::restoreState(const ErrorSeverityMapperState & saved)\n {\n-    while (localOnWarnings.ordinality() > saved.onWarningMax)\n-        localOnWarnings.pop();\n-    firstLocalOnWarning = saved.firstOnWarning;\n+    severityMappings.trunc(saved.maxMappings);\n+    firstActiveMapping = saved.firstActiveMapping;\n     activeSymbol = saved.symbol;\n }\n \n-void WarningProcessor::report(IErrorReceiver & errors)\n+IECLError * ErrorSeverityMapper::mapError(IECLError * error)\n {\n-    applyGlobalOnWarning();\n-    combineSandboxWarnings();\n-    if (allErrors.ordinality())\n-        reportErrors(errors, allErrors);\n-    else\n-        reportErrors(errors, warnings);\n+    //An error that is fatal cannot be mapped.\n+    Owned<IECLError> mappedError = IndirectErrorReceiver::mapError(error);\n+    if (!isFatal(mappedError))\n+    {\n+        //This takes precedence over mappings in the parent\n+        ErrorSeverity newSeverity = getWarningAction(mappedError->errorCode(), severityMappings, firstActiveMapping, SeverityUnknown);\n+        if (newSeverity != SeverityUnknown)\n+            return mappedError->cloneSetSeverity(newSeverity);\n+    }\n+    return mappedError.getClear();\n }\n \n-void WarningProcessor::report(IErrorReceiver * errors, IErrorReceiver * warnings, IECLError * warning)\n-{\n-    IAtom * action = getWarningAction(warning->errorCode(), localOnWarnings, firstLocalOnWarning);\n+//---------------------------------------------------------------------------------------------------------------------\n \n-    if (action == defaultAtom)\n-        action = getWarningAction(warning->errorCode(), globalOnWarnings, 0);\n+bool isGlobalOnWarning(IHqlExpression * expr)\n+{\n+    return ((expr->getOperator() == no_setmeta) && (expr->queryChild(0)->queryName() == onWarningAtom));\n+}\n \n-    if (action == defaultAtom)\n-        action = warning->isError() ? errorAtom : warningAtom;\n+//---------------------------------------------------------------------------\n \n-    if ((action == warningAtom) || (action == defaultAtom))\n+static HqlTransformerInfo globalOnWarningCollectorInfo(\"GlobalOnWarningCollector\");\n+class GlobalOnWarningCollector : public QuickHqlTransformer\n+{\n+public:\n+    GlobalOnWarningCollector(ErrorSeverityMapper & _mapper) :\n+        QuickHqlTransformer(globalOnWarningCollectorInfo, NULL), mapper(_mapper)\n     {\n-        if (warnings)\n-            warnings->report(warning);\n     }\n-    else if (action == errorAtom)\n+\n+    virtual void doAnalyse(IHqlExpression * expr)\n     {\n-        Owned<IECLError> error = changeErrorType(true, warning);\n-        if (errors)\n-            errors->report(error);\n-        else\n-            throw error.getClear();\n+        IHqlExpression * body = expr->queryBody();\n+        if (isGlobalOnWarning(body))\n+            mapper.addOnWarning(body);\n+        QuickHqlTransformer::doAnalyse(body);\n     }\n-}\n+\n+protected:\n+    ErrorSeverityMapper & mapper;\n+};\n \n \n-void WarningProcessor::combineSandboxWarnings()\n+static HqlTransformerInfo warningCollectingTransformerInfo(\"WarningCollectingTransformer\");\n+class WarningCollectingTransformer : public QuickHqlTransformer\n {\n-    StringBuffer s;\n-    ForEachItemInRev(i, warnings)\n-    {\n-        IECLError & cur = warnings.item(i);\n-        if (cur.errorCode() == WRN_DEFINITION_SANDBOXED)\n-        {\n-            if (s.length())\n-                s.append(\", \");\n-            s.append(cur.getFilename());\n-            warnings.remove(i);\n-        }\n-    }\n-    if (s.length())\n+public:\n+    WarningCollectingTransformer(IErrorReceiver & _errs) :\n+        QuickHqlTransformer(warningCollectingTransformerInfo, &_errs), mapper(_errs)\n     {\n-        s.insert(0, \"The following definitions are sandboxed: \");\n-        warnings.append(* createECLWarning(WRN_DEFINITION_SANDBOXED, s.str(), NULL, 0, 0, 0));\n     }\n-}\n \n-void WarningProcessor::applyGlobalOnWarning()\n-{\n-    ForEachItemIn(i, possibleWarnings)\n+    virtual void doAnalyse(IHqlExpression * expr)\n     {\n-        IECLError & cur = possibleWarnings.item(i);\n-        IAtom * action = getWarningAction(cur.errorCode(), globalOnWarnings, 0);\n-        if (action == defaultAtom || action == warningAtom)\n+        switch (expr->getAnnotationKind())\n         {\n-            if (cur.isError())\n-                appendUnique(allErrors, &cur);\n-            else\n-                appendUnique(warnings, &cur);\n+        case annotate_meta:\n+            {\n+                unsigned max = mapper.processMetaAnnotation(expr);\n+                QuickHqlTransformer::doAnalyse(expr);\n+                mapper.restoreLocalOnWarnings(max);\n+                return;\n+            }\n+        case annotate_warning:\n+            {\n+                IECLError * error = static_cast<CHqlWarningAnnotation *>(expr)->queryWarning();\n+                Owned<IECLError> mappedError = mapper.mapError(error);\n+                mapper.report(mappedError);\n+                break;\n+            }\n+        case annotate_symbol:\n+            {\n+                ErrorSeverityMapper::SymbolScope saved(mapper, expr);","path":"ecl/hql/hqlutil.cpp","position":454,"original_position":454,"commit_id":"957e447d9403ec5e015de2fdc0b4f895564f2132","original_commit_id":"957e447d9403ec5e015de2fdc0b4f895564f2132","user":{"login":"ghalliday","id":902490,"avatar_url":"https://gravatar.com/avatar/402dddc31c0e131326457e9ffd436352?d=https%3A%2F%2Fidenticons.github.com%2Ffe5515252d652398f3855425ad2a507e.png&r=x","gravatar_id":"402dddc31c0e131326457e9ffd436352","url":"https://api.github.com/users/ghalliday","html_url":"https://github.com/ghalliday","followers_url":"https://api.github.com/users/ghalliday/followers","following_url":"https://api.github.com/users/ghalliday/following{/other_user}","gists_url":"https://api.github.com/users/ghalliday/gists{/gist_id}","starred_url":"https://api.github.com/users/ghalliday/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ghalliday/subscriptions","organizations_url":"https://api.github.com/users/ghalliday/orgs","repos_url":"https://api.github.com/users/ghalliday/repos","events_url":"https://api.github.com/users/ghalliday/events{/privacy}","received_events_url":"https://api.github.com/users/ghalliday/received_events","type":"User","site_admin":false},"body":"It saves the current state of the onwarning processing.  Local onWarnings apply from the point they are seen to the next definition.  This object prevents them being applied any more, but restores the processing once the object goes out of scope.\r\n","created_at":"2014-03-11T09:14:52Z","updated_at":"2014-03-11T09:14:52Z","html_url":"https://github.com/hpcc-systems/HPCC-Platform/pull/5436#discussion_r10465787","pull_request_url":"https://api.github.com/repos/hpcc-systems/HPCC-Platform/pulls/5436","_links":{"self":{"href":"https://api.github.com/repos/hpcc-systems/HPCC-Platform/pulls/comments/10465787"},"html":{"href":"https://github.com/hpcc-systems/HPCC-Platform/pull/5436#discussion_r10465787"},"pull_request":{"href":"https://api.github.com/repos/hpcc-systems/HPCC-Platform/pulls/5436"}}}}