[PATCH] D99286: [AsmParser][SystemZ][z/OS] Add in support to only use CommentString as a possible comment syntax

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Apr 4 11:45:27 PDT 2021


MaskRay added a comment.

In D99286#2668227 <https://reviews.llvm.org/D99286#2668227>, @anirudhp wrote:

> In D99286#2667966 <https://reviews.llvm.org/D99286#2667966>, @MaskRay wrote:
>
>> If the C and BCPL style comments don't cause conflict with HLASM, I'd rather we don't add any code. We don't necessarily reject syntax which are unlikely used incorrectly by the user.
>
> They do cause "conflict". HLASM currently does not treat the additional comment strings as "comments".

This does not necessarily mean a conflict. As an concrete example, ld.lld treats `#` as an additional comment marker for linker scripts. GNU ld don't do this.
The ld.lld way can be seen as an extension. Since there is no meaningful starting with `#` in GNU ld linker scripts, reserving `#` for an additional comment marker is totally fine.

My question here is similar. If there is no meaningful syntax starting with `/*` or `//`, we don't necessarily detect such cases.

>> If such distinction is really needed, maybe name the variable with a positive meaning, e.g. AllowCCommentMarker, instead of Only* (Disallow*).
>
> How about a new field called `AllowAdditionalComments` instead? This will encapsulate allowing the C-style line and block comments, and the "#" string as additional comment strings in addition to the CommentString attribute.

Looks good.

Note that `#` is special - some dialects don't use it for comment markers at an arbitrary column but allows it at the line beginning.
If HLASM does not use `#` at line beginning as useful syntax, we don't necessarily detect it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99286/new/

https://reviews.llvm.org/D99286



More information about the llvm-commits mailing list