[constraint-name-style] crashes on out-of-line constraint definition

[fangism]

Test case:

constraint classname::constraint_c {
  a <= b;
}

crashes:

F0220 14:20:14.389087  204922 identifier.cc:74] Check failed: NodeEnum(t.tag) == NodeEnum::kUnqualifiedId (kQualifiedId vs. kUnqualifiedId) 
*** Check failure stack trace: ***
    @     0x555ff8c4dfbf  absl::logging_internal::LogMessage::DieIfFatal()
    @     0x555ff8c4d6e6  absl::logging_internal::LogMessage::Flush()
    @     0x555ff8c4f3e9  absl::logging_internal::LogMessageFatal::~LogMessageFatal()
    @     0x555ff8c00191  verilog::AutoUnwrapIdentifier()
    @     0x555ff8be7920  verilog::GetSymbolIdentifierFromConstraintDeclaration()
    @     0x555ff8be72e9  verilog::analysis::ConstraintNameStyleRule::HandleSymbol()
    @     0x555ff8be0613  verible::SyntaxTreeLinter::Visit()
    @     0x555ff8c0316a  verible::TreeContextVisitor::Visit()
    @     0x555ff8c0316a  verible::TreeContextVisitor::Visit()
    @     0x555ff8c0316a  verible::TreeContextVisitor::Visit()
    @     0x555ff8c0316a  verible::TreeContextVisitor::Visit()
    @     0x555ff8be0345  verible::SyntaxTreeLinter::Lint()
    @     0x555ff8bb11ec  verilog::VerilogLintTextStructure()
    @     0x555ff8bb0d16  verilog::LintOneFile()
    @     0x555ff8baf47f  main
    @     0x7f6860279bbd  __libc_start_main
    @     0x555ff8baede9  _start
*** SIGABRT received by PID 204922 (TID 204922) from PID 204922; ***
F0220 14:20:14.389087  204922 identifier.cc:74] Check failed: NodeEnum(t.tag) == NodeEnum::kUnqualifiedId (kQualifiedId vs. kUnqualifiedId) 
E0220 14:20:14.400606  204922 process_state.cc:639] RAW: Raising signal 6 with default behavior

[fangism]

b/149945032

[fangism]

@tgorochowik since you implemented the original rule, could you take a look?

[fangism]

Recommendation: since an out-of-line definition is always followed by a forward declaration somewhere else (in this case inside a class), you could just ignore all out-of-line definitions (whose "names" are qualified) to avoid duplicate lint errors on the same name.

[tgorochowik]

@fangism I am thinking about what would be the best way to do that.

Would that be okay to make GetSymbolIdentifierFromConstraintDeclaration return a pointer instead of a reference and make it return nullptr if it's dealing with an out-of-line definition? That would make the implementation pretty simple.

Ended up just adding an extra CST function to check whether a kConstraintDeclaration is an out-of-line definition.

[fangism]

@fangism I am thinking about what would be the best way to do that.

~Would that be okay to make GetSymbolIdentifierFromConstraintDeclaration return a pointer instead of a reference and make it return nullptr if it's dealing with an out-of-line definition? That would make the implementation pretty simple.~

Ended up just adding an extra CST function to check whether a kConstraintDeclaration is an out-of-line definition.

Simple enough.