[THRIFT-3709] Comment syntax can produce broken code

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 0.10.0
• Component/s: Compiler (General)
• Patch Info: Patch Available
• Assignee: Jens Geyer
• Reporter: Tom Davis
• Created: 04/Mar/16 19:46
• Updated: 20/Feb/21 15:27
• Resolved: 06/Mar/16 23:21

Description

Somewhere between 85650612e15c79c79e470553d3779d18f755150c and 6ec6860801bdc87236e636add071c4faa2ac7e4b the handling of comments was changed. This change appears to have removed support for the syntax /** Comment **/. In particular, the comment-end token can't have more than one asterisk.

However, it is possible to "trick" the parser by having multiple comments:

struct Thing {
  /** This is a comment */
  1: required string id,
  /** This is also a comment **/
  2: required string typeId,
  /** Yet another comment! */
  3: required i32 endTimestamp
}

In this case, the IDL file will parse correctly but will produce broken code, at least in the case of the Java compiler. Here is the relevant part of the generated file, first from the older commit (functional), then from master (broken).

older commit (expected result)

  /**
   * This is a comment
   */
  public String id; // required
  /**
   * This is also a comment *
   */
  public String typeId; // required
  /**
   * Yet another comment!
   */
  public int endTimestamp; // required

current master (broken)

  /**
   * This is a comment
   */
  public String id; // required
  /**
   * This is also a comment **/
   * 2: required string typeId,
   * /** Yet another comment!
   */
  public int endTimestamp; // required

The line * 2: required string typeId, lives outside the comment so the code fails to compile.

For my part, I'll modify my IDL files to only use the single-asterisk syntax. However, I would expect generation to fail similarly to how it does with a single /** Comment **/ (with a maybe-not-totally-helpful "unexpected token" error)

Issue Links

• relates to: THRIFT-3501 Forward slash in comment causes compiler error
• relates to: THRIFT-3528 Fix warnings in thrift.ll