[PATCH] D43390: [DEBUGINFO] Add support for emission of the inlined strings.

[Paul Robinson via Phabricator via llvm-commits]

probinson added inline comments.


================
Comment at: test/DebugInfo/Generic/inlined-strings.ll:38
+
+; CHECK-NOT: DW_form_str
+
----------------
probinson wrote:
> ABataev wrote:
> > probinson wrote:
> > > This is spelled incorrectly (I expect you want DW_FORM_strp), and only verifies that the form does not appear after the last positive matching string.
> > > 
> > > I would be more inclined to run the test twice, once with Enable and once with Disable, with checks along the lines of
> > > ```
> > > ENABLE-NOT: DW_FORM_str{{(p|x)}}
> > > ENABLE: DW_FORM_string
> > > ENABLE-NOT: DW_FORM_str{{(p|x)}}
> > > 
> > > DISABLE-NOT: DW_FORM_string
> > > DISABLE: DW_FORM_str{{p|x)}}
> > > DISABLE-NOT: DW_FORM_string
> > > ```
> > > That makes the test specific to the behavior of the option, and the forms it produces, omitting irrelevant concerns about tags and attributes.
> > 1. I don't care about some particular form here, I'm just checking that there are no any strings at all, in any form.
> > 2. There are a lot of tests already where this new option is disabled by default and they have checks for `DW_form_strp` and other forms.
> 1. Actually you want to specifically verify that you have at least one string, that it uses DW_FORM_string, and that no strings use any of the indirect forms.
> 2. Fair.  In that case, just do:
> ```
> CHECK-NOT: DW_FORM_str{{(p|x)}}
> CHECK: DW_FORM_string
> CHECK-NOT: DW_FORM_str{{(p|x)}}
> ```
> 
Actually I meant the above 3 CHECK lines were all you needed, for the entire test.  Sorry for not making that clear.


Repository:
  rL LLVM

https://reviews.llvm.org/D43390