Skip to content

Commit 3dec46d

Browse files
authored
[HLSL][RootSignature] Correct RootSignatureParser to use correct SourceLocation in diagnostics (#147084)
The `SourceLocation` of a `RootSignatureToken` is incorrectly set to be the "offset" into the concatenated string that denotes the rootsignature. This causes an issue when the `StringLiteral` is a multi-line expansion macro, since the offset will not account for the characters between `StringLiteral` tokens. This pr resolves this by retaining the `SourceLocation` information that is kept in `StringLiteral` and then converting the offset in the concatenated string into the proper `SourceLocation` using the `StringLiteral::getLocationOfByte` interface. To do so, we will need to adjust the `RootSignatureToken` to only hold its offset into the root signature string. Then when the parser will use the token, it will need to compute its actual `SourceLocation`. See linked issue for more context. For example: ``` #define DemoRootSignature \ "CBV(b0)," \ "RootConstants(num32BitConstants = 3, b0, invalid)" expected caret location ---------------^ actual caret location ------------^ ``` The caret points 5 characters early because the current offset did not account for the characters: ``` '"' ' ' '\' ' ' '"' 1 2 3 4 5 ``` - Updates `RootSignatureParser` to retain `SourceLocation` information by retaining the `StringLiteral` and passing the underlying `StringRef` to the `Lexer` - Updates `RootSignatureLexer` so that the constructed tokens only reflect an offset into the `StringRef` - Updates `RootSignatureParser` to directly construct its used `Lexer` so that the `StringLiteral` is directly tied with the string used in the `RootSignatureLexer` - Updates `RootSignatureParser` to use `StringLiteral::getLocationOfByte` to get the actual token location for diagnostics - Updates `ParseHLSLRootSignatureTest` to construct a phony `AST`/`StringLiteral` for the test cases - Adds a test to `RootSignature-err.hlsl` showing that the `SourceLocation` is correctly set for diagnostics in a multi-line macro expansion Resolves: #146967
1 parent 36dbe51 commit 3dec46d

File tree

8 files changed

+265
-239
lines changed

8 files changed

+265
-239
lines changed

clang/include/clang/Lex/LexHLSLRootSignature.h

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,17 @@ struct RootSignatureToken {
3131

3232
Kind TokKind = Kind::invalid;
3333

34-
// Retain the SouceLocation of the token for diagnostics
35-
clang::SourceLocation TokLoc;
34+
// Retain the location offset of the token in the Signature
35+
// string
36+
uint32_t LocOffset;
3637

3738
// Retain spelling of an numeric constant to be parsed later
3839
StringRef NumSpelling;
3940

4041
// Constructors
41-
RootSignatureToken(clang::SourceLocation TokLoc) : TokLoc(TokLoc) {}
42-
RootSignatureToken(Kind TokKind, clang::SourceLocation TokLoc)
43-
: TokKind(TokKind), TokLoc(TokLoc) {}
42+
RootSignatureToken(uint32_t LocOffset) : LocOffset(LocOffset) {}
43+
RootSignatureToken(Kind TokKind, uint32_t LocOffset)
44+
: TokKind(TokKind), LocOffset(LocOffset) {}
4445
};
4546

4647
inline const DiagnosticBuilder &
@@ -61,8 +62,7 @@ operator<<(const DiagnosticBuilder &DB, const RootSignatureToken::Kind Kind) {
6162

6263
class RootSignatureLexer {
6364
public:
64-
RootSignatureLexer(StringRef Signature, clang::SourceLocation SourceLoc)
65-
: Buffer(Signature), SourceLoc(SourceLoc) {}
65+
RootSignatureLexer(StringRef Signature) : Buffer(Signature) {}
6666

6767
/// Consumes and returns the next token.
6868
RootSignatureToken consumeToken();
@@ -76,23 +76,21 @@ class RootSignatureLexer {
7676
}
7777

7878
private:
79-
// Internal buffer to iterate over
79+
// Internal buffer state
8080
StringRef Buffer;
81+
uint32_t LocOffset = 0;
8182

8283
// Current peek state
8384
std::optional<RootSignatureToken> NextToken = std::nullopt;
8485

85-
// Passed down parameters from Sema
86-
clang::SourceLocation SourceLoc;
87-
8886
/// Consumes the buffer and returns the lexed token.
8987
RootSignatureToken lexToken();
9088

9189
/// Advance the buffer by the specified number of characters.
9290
/// Updates the SourceLocation appropriately.
9391
void advanceBuffer(unsigned NumCharacters = 1) {
9492
Buffer = Buffer.drop_front(NumCharacters);
95-
SourceLoc = SourceLoc.getLocWithOffset(NumCharacters);
93+
LocOffset += NumCharacters;
9694
}
9795
};
9896

clang/include/clang/Parse/ParseHLSLRootSignature.h

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#ifndef LLVM_CLANG_PARSE_PARSEHLSLROOTSIGNATURE_H
1414
#define LLVM_CLANG_PARSE_PARSEHLSLROOTSIGNATURE_H
1515

16+
#include "clang/AST/Expr.h"
1617
#include "clang/Basic/DiagnosticParse.h"
1718
#include "clang/Lex/LexHLSLRootSignature.h"
1819
#include "clang/Lex/Preprocessor.h"
@@ -29,7 +30,7 @@ class RootSignatureParser {
2930
public:
3031
RootSignatureParser(llvm::dxbc::RootSignatureVersion Version,
3132
SmallVector<llvm::hlsl::rootsig::RootElement> &Elements,
32-
RootSignatureLexer &Lexer, clang::Preprocessor &PP);
33+
StringLiteral *Signature, Preprocessor &PP);
3334

3435
/// Consumes tokens from the Lexer and constructs the in-memory
3536
/// representations of the RootElements. Tokens are consumed until an
@@ -187,12 +188,23 @@ class RootSignatureParser {
187188
bool tryConsumeExpectedToken(RootSignatureToken::Kind Expected);
188189
bool tryConsumeExpectedToken(ArrayRef<RootSignatureToken::Kind> Expected);
189190

191+
/// Convert the token's offset in the signature string to its SourceLocation
192+
///
193+
/// This allows to currently retrieve the location for multi-token
194+
/// StringLiterals
195+
SourceLocation getTokenLocation(RootSignatureToken Tok);
196+
197+
/// Construct a diagnostics at the location of the current token
198+
DiagnosticBuilder reportDiag(unsigned DiagID) {
199+
return getDiags().Report(getTokenLocation(CurToken), DiagID);
200+
}
201+
190202
private:
191203
llvm::dxbc::RootSignatureVersion Version;
192204
SmallVector<llvm::hlsl::rootsig::RootElement> &Elements;
193-
RootSignatureLexer &Lexer;
194-
195-
clang::Preprocessor &PP;
205+
StringLiteral *Signature;
206+
RootSignatureLexer Lexer;
207+
Preprocessor &PP;
196208

197209
RootSignatureToken CurToken;
198210
};

clang/lib/Lex/LexHLSLRootSignature.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@ RootSignatureToken RootSignatureLexer::lexToken() {
2727
advanceBuffer(Buffer.take_while(isspace).size());
2828

2929
if (isEndOfBuffer())
30-
return RootSignatureToken(TokenKind::end_of_stream, SourceLoc);
30+
return RootSignatureToken(TokenKind::end_of_stream, LocOffset);
3131

3232
// Record where this token is in the text for usage in parser diagnostics
33-
RootSignatureToken Result(SourceLoc);
33+
RootSignatureToken Result(LocOffset);
3434

3535
char C = Buffer.front();
3636

@@ -62,7 +62,7 @@ RootSignatureToken RootSignatureLexer::lexToken() {
6262

6363
// All following tokens require at least one additional character
6464
if (Buffer.size() <= 1) {
65-
Result = RootSignatureToken(TokenKind::invalid, SourceLoc);
65+
Result = RootSignatureToken(TokenKind::invalid, LocOffset);
6666
return Result;
6767
}
6868

clang/lib/Parse/ParseDeclCXX.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4941,20 +4941,16 @@ void Parser::ParseHLSLRootSignatureAttributeArgs(ParsedAttributes &Attrs) {
49414941
}
49424942

49434943
// Construct our identifier
4944-
StringRef Signature = StrLiteral.value()->getString();
4944+
StringLiteral *Signature = StrLiteral.value();
49454945
auto [DeclIdent, Found] =
4946-
Actions.HLSL().ActOnStartRootSignatureDecl(Signature);
4946+
Actions.HLSL().ActOnStartRootSignatureDecl(Signature->getString());
49474947
// If we haven't found an already defined DeclIdent then parse the root
49484948
// signature string and construct the in-memory elements
49494949
if (!Found) {
4950-
// Offset location 1 to account for '"'
4951-
SourceLocation SignatureLoc =
4952-
StrLiteral.value()->getExprLoc().getLocWithOffset(1);
49534950
// Invoke the root signature parser to construct the in-memory constructs
4954-
hlsl::RootSignatureLexer Lexer(Signature, SignatureLoc);
49554951
SmallVector<llvm::hlsl::rootsig::RootElement> RootElements;
49564952
hlsl::RootSignatureParser Parser(getLangOpts().HLSLRootSigVer, RootElements,
4957-
Lexer, PP);
4953+
Signature, PP);
49584954
if (Parser.parse()) {
49594955
T.consumeClose();
49604956
return;

0 commit comments

Comments
 (0)