oasis-tcs/sarif-spec

Clarify use (or extend design) of SARIF to express hierarchical diagnostics.

michaelcfanning opened this issue · 20 comments

Clarify use (or extend design) of SARIF to express hierarchical diagnostics.

Context: in Visual Studio we're working on hierarchical diagnostics to present compiler errors in a more structured way than plain text. Clang is also working on a similar project.

A C++ example:

struct dog{};
struct cat{};

void pet(dog);
void pet(cat);

template <class T>
concept pettable = requires(T t) { t.pet(); };

template <pettable T>
void pet(T);

struct donkey {};

int main() {
    pet(donkey{});
}

MSVC outputs an error message like this in plain text:

error C2665: 'pet': no overloaded function could convert all the argument types
message : could be 'void pet(cat)'
message : 'void pet(cat)': cannot convert argument 1 from 'donkey' to 'cat'
message : No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called
message : or       'void pet(dog)'
message : 'void pet(dog)': cannot convert argument 1 from 'donkey' to 'dog'
message : No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called
message : or       'void pet(T)'
message : the associated constraints are not satisfied
message : the concept 'pettable<donkey>' evaluated to false
message : 'pet': is not a member of 'donkey'
message : see declaration of 'donkey'
message : while trying to match the argument list '(donkey)'

However, there's a logical structure which isn't represented here, which looks something like:

  • error C2665: 'pet': no overloaded function could convert all the argument types
    • could be 'void pet(cat)'
      • 'void pet(cat)': cannot convert argument 1 from 'donkey' to 'cat'
        • No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called
    • or 'void pet(dog)'
      • 'void pet(dog)': cannot convert argument 1 from 'donkey' to 'dog'
        • No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called
    • or 'void pet(T)'
      • the associated constraints are not satisfied
        • the concept 'pettable' evaluated to false
          • 'pet': is not a member of 'donkey'
          • see declaration of 'donkey'
        • while trying to match the argument list '(donkey)'

In order to represent this hierarchy, we want to be able to encode it in SARIF. However, there's currently no facility for this.

The way we do this today is by placing the nested diagnostics in a relatedLocations object. To each related location, we add a properties property which contains a nestingLevel property. This is essentially the same as the nestingLevel property of threadFlowLocation objects ([3.38.10]).

For example:

{
   "ruleId":"C2665",
   "level":"error",
   "message":{
      "text":"'pet': no overloaded function could convert all the argument types"
   },
   "locations":[
      {
         "physicalLocation":{
            "artifactLocation":{
               "uri":"diag.cpp"
            },
            "region":{
               "startLine":35,
               "startColumn":4
            }
         }
      }
   ],
   "relatedLocations":[
      {
         "id":0,
         "physicalLocation":{
            "artifactLocation":{
               "uri":"diag.cpp"
            },
            "region":{
               "startLine":11,
               "startColumn":5
            }
         },
         "message":{
            "text":"could be 'void __cdecl pet(struct dog)'"
         }
      },
      {
         "id":1,
         "physicalLocation":{
            "artifactLocation":{
               "uri":"diag.cpp"
            },
            "region":{
               "startLine":35,
               "startColumn":4
            }
         },
         "message":{
            "text":"'void __cdecl pet(struct dog)': cannot convert argument 1 from 'int' to 'struct dog'"
         },
         "properties":{
            "nestingLevel":1
         }
      },
      {
         "id":2,
         "physicalLocation":{
            "artifactLocation":{
               "uri":"diag.cpp"
            },
            "region":{
               "startLine":35,
               "startColumn":8
            }
         },
         "message":{
            "text":"No constructor could take the source type, or constructor overload resolution was ambiguous"
         },
         "properties":{
            "nestingLevel":1
         }
      }
   ]
}

I'd like for all C++ compilers to agree on how to encode hierarchical diagnostics, and having it standardised in SARIF seems to be the best way to achieve this.

Sorry for the delay on this topic. We are discussing at tomorrow's TC meeting.

There's some appetite in the ISO C++ standards group for converging on SARIF with an extension like this. Has this item been discussed by the SARIF group yet?