D:\a\1\s\onnx\defs\parser.cc(72,7): warning C4101: 'e': unreferenced local variable [D:\a\1\s.setuptools-cmake-build\onnx.vcxproj]

https://dev.azure.com/onnx-pipelines/onnx/_build/results?buildId=51364&view=logs&j=ce189759-b1eb-58fb-a270-a52102be8778&t=9db78ef0-98f3-5f2e-158d-66f6d2a6591b&l=596

@gramalingam it seems like the parser treats any attribute value starting with an isalpha character as tensor, graph, or proto type, see https://github.com/onnx/onnx/blob/main/onnx/defs/parser.cc#L421. So I can't really discriminate against inf, infinity, or nan at the ::Parse(Literal&) level. Not sure how to proceed here. Should I add specific checking for valid float alphabetic literals in ParseSingleAttributeValue? Are we explicitly banning the use of inf, nan, and infinity as anything but float literals?

the parser treats any attribute value starting with an isalpha character as tensor, graph, or proto type

Hmmm, that's a good point. The tensor/proto is not a concern, because that is a fixed list of type-identifiers that does not clash with inf/nan. The concern is the graph-name, specifically if users want to give a graph-attribute graph a name like "inf".

I think checking for a valid float alphabetic literal is a reasonable choice. This does not ban the use of "inf" etc. as a valid name in other places (like a tensor variable name), it only restricts its use as a graph-name for a graph-attribute. Given that we are already restricting the graph-identifier (it can't use type names like "float"), this may be okay.

Further, we can provide an "escape mechanism" to allow users who really want to produce a graph with name "inf" ... users can write attributes with or without type-annotation: e.g., they can write "mygraph : graph = ..." or "mygraph = ...". It is only in the latter situation that we need to make a guess. In the former case, we can use the type-annotation to guide the parser (which doesn't exist currently, but we can add it easily enough).

Does that sound reasonable? Thanks!

I believe everything has been addressed. Please take another look. Thanks for the reviews!