Skip to content

Commit 68498ad

Browse files
committed
(PUP-7224) Replace Rgen AST model with one generated from Pcore
This commit adds the ast.rb file (generated from the ast.pp using the previously added rake task "gen_pcore_ast") and removes the Rgen based predecessor. Since the new model is immutable, this has consequences on two major areas. One is how offset, length, and the locator is handled and the other relates to how containers are accessed. In the old model, all expressions had a reference to their parent. This is very hard to achieve in a tree of immutable elements (parent would need to exist before the children that it is supposed to contain are created). In the new model, there is instead one common element that is referenced from every other element, and that is the Locator. Keeping a reference to the Locator is just a cheap as keeping a reference to a parent and it has the additional advantage of completely removing the need for SourcePosAdapters. An element that knows its locator, offset, and length, can respond directly to #file, #line, #pos using derived attributes. The lack of parent also prompted a change in the validator (it often consults the container of elements, and at times, needs to travererse all the way to the root). The solution for this was to let the corresponds to the current parent chain). That stack makes it possible for the validator to navigate the parents at all times.
1 parent edb930a commit 68498ad

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+6046
-1895
lines changed

Diff for: lib/puppet/info_service/class_information_service.rb

+1-2
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@ def value_as_literal(value_expr)
105105

106106
# Extracts the source for the expression
107107
def extract_value_source(value_expr)
108-
position = Puppet::Pops::Adapters::SourcePosAdapter.adapt(value_expr)
109-
position.extract_tree_text
108+
value_expr.locator.extract_tree_text(value_expr)
110109
end
111110
end

Diff for: lib/puppet/parser/ast/pops_bridge.rb

+2-2
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ def instantiate_HostClassDefinition(o, modname)
207207

208208
def instantiate_ResourceTypeDefinition(o, modname)
209209
instance = Puppet::Resource::Type.new(:definition, o.name, @context.merge(args_from_definition(o, modname, ExpressionSupportingReturn)))
210-
Puppet::Pops::Loaders.register_runtime3_type(instance.name, Puppet::Pops::Adapters::SourcePosAdapter.adapt(o).to_uri)
210+
Puppet::Pops::Loaders.register_runtime3_type(instance.name, o.locator.to_uri(o))
211211
instance
212212
end
213213

@@ -269,7 +269,7 @@ def instantiate_FunctionDefinition(function_definition, modname)
269269

270270
# Instantiate Function, and store it in the loader
271271
typed_name, f = Puppet::Pops::Loader::PuppetFunctionInstantiator.create_from_model(function_definition, loader)
272-
loader.set_entry(typed_name, f, Puppet::Pops::Adapters::SourcePosAdapter.adapt(function_definition).to_uri)
272+
loader.set_entry(typed_name, f, function_definition.locator.to_uri(function_definition))
273273

274274
nil # do not want the function to inadvertently leak into 3x
275275
end

Diff for: lib/puppet/pops.rb

+3-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ module Pops
3939
require 'puppet/pops/label_provider'
4040
require 'puppet/pops/validation'
4141
require 'puppet/pops/issue_reporter'
42-
require 'puppet/pops/model/model'
4342

4443
require 'puppet/pops/time/timespan'
4544
require 'puppet/pops/time/timestamp'
@@ -52,6 +51,7 @@ module Pops
5251
require 'puppet/pops/merge_strategy'
5352

5453
module Model
54+
require 'puppet/pops/model/ast'
5555
require 'puppet/pops/model/tree_dumper'
5656
require 'puppet/pops/model/ast_transformer'
5757
require 'puppet/pops/model/factory'
@@ -149,4 +149,6 @@ module Serialization
149149
require 'puppet/bindings'
150150
require 'puppet/functions'
151151
require 'puppet/loaders'
152+
153+
Puppet::Pops::Model.register_pcore_types
152154
end

Diff for: lib/puppet/pops/adapters.rb

+14-84
Original file line numberDiff line numberDiff line change
@@ -11,96 +11,32 @@ class DocumentationAdapter < Adaptable::Adapter
1111
attr_accessor :documentation
1212
end
1313

14-
# A SourcePosAdapter holds a reference to a *Positioned* object (object that has offset and length).
15-
# This somewhat complex structure makes it possible to correctly refer to a source position
16-
# in source that is embedded in some resource; a parser only sees the embedded snippet of source text
17-
# and does not know where it was embedded. It also enables lazy evaluation of source positions (they are
18-
# rarely needed - typically just when there is an error to report.
19-
#
20-
# @note It is relatively expensive to compute line and position on line - it is not something that
21-
# should be done for every token or model object.
22-
#
23-
# @see Utils#find_adapter, Utils#find_closest_positioned
24-
#
25-
class SourcePosAdapter < Adaptable::Adapter
26-
attr_accessor :locator
27-
attr_reader :adapted
28-
29-
def self.create_adapter(o)
30-
new(o)
31-
end
32-
33-
def initialize(o)
34-
@adapted = o
14+
# This class is for backward compatibility only. It's not really an adapter but it is
15+
# needed for the puppetlabs-strings gem
16+
# @deprecated
17+
class SourcePosAdapter
18+
def self.adapt(object)
19+
new(object)
3520
end
3621

37-
def locator
38-
# The locator is always the parent locator, all positioned objects are positioned within their
39-
# parent. If a positioned object also has a locator that locator is for its children!
40-
#
41-
@locator ||= self.class.find_locator(@adapted.eContainer)
22+
def initialize(object)
23+
@object = object
4224
end
4325

44-
# @api private
45-
def self.find_locator(o)
46-
raise ArgumentError, 'InternalError: SourcePosAdapter for something that has no locator among parents' if o.nil?
47-
found_locator = o.respond_to?(:locator) ? o.locator : nil
48-
return found_locator unless found_locator.nil?
49-
adapter = get(o)
50-
return adapter.locator unless adapter.nil?
51-
container = o.eContainer
52-
container.nil? ? nil : find_locator(container)
53-
end
54-
55-
def offset
56-
@adapted.offset
26+
def file
27+
@object.file
5728
end
5829

59-
def length
60-
@adapted.length
61-
end
62-
63-
# Produces the line number for the given offset.
64-
# @note This is an expensive operation
65-
#
6630
def line
67-
locator.line_for_offset(offset)
31+
@object.line
6832
end
6933

70-
# Produces the position on the line of the given offset.
71-
# @note This is an expensive operation
72-
#
7334
def pos
74-
locator.pos_on_line(offset)
35+
@object.pos
7536
end
7637

77-
# Extracts the text represented by this source position (the string is obtained from the locator)
7838
def extract_text
79-
locator.extract_text(offset, length)
80-
end
81-
82-
def extract_tree_text
83-
first = @adapted.offset
84-
last = first + @adapted.length
85-
@adapted.eAllContents.each do |m|
86-
m_offset = m.offset
87-
next if m_offset.nil?
88-
first = m_offset if m_offset < first
89-
m_last = m_offset + m.length
90-
last = m_last if m_last > last
91-
end
92-
locator.extract_text(first, last-first)
93-
end
94-
95-
# Produces an URI with path?line=n&pos=n. If origin is unknown the URI is string:?line=n&pos=n
96-
def to_uri
97-
f = locator.file
98-
if f.nil? || f.empty?
99-
f = 'string:'
100-
else
101-
f = Puppet::Util.path_to_uri(f).to_s
102-
end
103-
URI("#{f}?line=#{line.to_s}&pos=#{pos.to_s}")
39+
@object.locator.extract_text(@object.offset, @object.length)
10440
end
10541
end
10642

@@ -149,7 +85,7 @@ class PathsAndNameCacheAdapter < Puppet::Pops::Adaptable::Adapter
14985
# @param instance
15086
# @api private
15187
def self.loader_name_by_source(environment, instance, file)
152-
file = find_file(instance) if file.nil?
88+
file = instance.file if file.nil?
15389
return nil if file.nil?
15490
pn_adapter = PathsAndNameCacheAdapter.adapt(environment) do |a|
15591
a.paths ||= environment.modulepath.map { |p| Pathname.new(p) }
@@ -181,12 +117,6 @@ def self.find_module_for_dir(environment, paths, dir)
181117
end
182118
nil
183119
end
184-
185-
# @api private
186-
def self.find_file(instance)
187-
source_pos = Utils.find_closest_positioned(instance)
188-
source_pos.nil? ? nil : source_pos.locator.file
189-
end
190120
end
191121
end
192122
end

Diff for: lib/puppet/pops/containment.rb

+12-12
Original file line numberDiff line numberDiff line change
@@ -84,19 +84,19 @@ def all_containment_getters(element)
8484
end
8585

8686
def collect_getters(eclass, containments)
87-
eclass.eStructuralFeatures.select {|r| r.is_a?(RGen::ECore::EReference) && r.containment}.each do |r|
88-
n = r.name
89-
containments << :"get#{n[0..0].upcase + ( n[1..-1] || "" )}"
87+
eclass.eStructuralFeatures.select {|r| r.is_a?(RGen::ECore::EReference) && r.containment}.each do |r|
88+
n = r.name
89+
containments << :"get#{n[0..0].upcase + ( n[1..-1] || "" )}"
90+
end
91+
eclass.eSuperTypes.each do |t|
92+
if cached = @@cache[ t.instanceClass ]
93+
containments.concat(cached)
94+
else
95+
super_containments = []
96+
collect_getters(t, super_containments)
97+
@@cache[ t.instanceClass ] = super_containments
98+
containments.concat(super_containments)
9099
end
91-
eclass.eSuperTypes.each do |t|
92-
if cached = @@cache[ t.instanceClass ]
93-
containments.concat(cached)
94-
else
95-
super_containments = []
96-
collect_getters(t, super_containments)
97-
@@cache[ t.instanceClass ] = super_containments
98-
containments.concat(super_containments)
99-
end
100100
end
101101
end
102102

Diff for: lib/puppet/pops/evaluator/collector_transformer.rb

+2-7
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,11 @@ def transform(o, scope)
2323

2424
fail "Resource type #{type} doesn't exist" unless resource_type
2525

26-
adapter = Adapters::SourcePosAdapter.adapt(o)
27-
line_num = adapter.line
28-
position = adapter.pos
29-
file_path = adapter.locator.file
30-
3126
if !o.operations.empty?
3227
overrides = {
3328
:parameters => o.operations.map{ |x| @@evaluator.evaluate(x, scope)}.flatten,
34-
:file => file_path,
35-
:line => [line_num, position],
29+
:file => o.file,
30+
:line => [o.line, o.pos],
3631
:source => scope.source,
3732
:scope => scope
3833
}

Diff for: lib/puppet/pops/evaluator/evaluator_impl.rb

+8-7
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
require 'rgen/ecore/ecore'
21
require 'puppet/parser/scope'
32
require 'puppet/pops/evaluator/compare_operator'
43
require 'puppet/pops/evaluator/relationship_operator'
@@ -376,7 +375,7 @@ def eval_ArithmeticExpression(o, scope)
376375
right = evaluate(o.right_expr, scope)
377376

378377
begin
379-
result = calculate(left, right, o.operator, o.left_expr, o.right_expr, scope)
378+
result = calculate(left, right, o, scope)
380379
rescue ArgumentError => e
381380
fail(Issues::RUNTIME_ERROR, o, {:detail => e.message}, e)
382381
end
@@ -386,11 +385,13 @@ def eval_ArithmeticExpression(o, scope)
386385

387386
# Handles binary expression where lhs and rhs are array/hash or numeric and operator is +, - , *, % / << >>
388387
#
389-
def calculate(left, right, operator, left_o, right_o, scope)
388+
def calculate(left, right, bin_expr, scope)
389+
operator = bin_expr.operator
390390
unless ARITHMETIC_OPERATORS.include?(operator)
391-
fail(Issues::UNSUPPORTED_OPERATOR, left_o.eContainer, {:operator => o.operator})
391+
fail(Issues::UNSUPPORTED_OPERATOR, bin_expr, {:operator => operator})
392392
end
393393

394+
left_o = bin_expr.left_expr
394395
if (left.is_a?(Array) || left.is_a?(Hash)) && COLLECTION_OPERATORS.include?(operator)
395396
# Handle operation on collections
396397
case operator
@@ -407,7 +408,7 @@ def calculate(left, right, operator, left_o, right_o, scope)
407408
else
408409
# Handle operation on numeric
409410
left = coerce_numeric(left, left_o, scope)
410-
right = coerce_numeric(right, right_o, scope)
411+
right = coerce_numeric(right, bin_expr.right_expr, scope)
411412
begin
412413
if operator == '%' && (left.is_a?(Float) || right.is_a?(Float))
413414
# Deny users the fun of seeing severe rounding errors and confusing results
@@ -430,7 +431,7 @@ def calculate(left, right, operator, left_o, right_o, scope)
430431
rescue NoMethodError => e
431432
fail(Issues::OPERATOR_NOT_APPLICABLE, left_o, {:operator => operator, :left_value => left})
432433
rescue ZeroDivisionError => e
433-
fail(Issues::DIV_BY_ZERO, right_o)
434+
fail(Issues::DIV_BY_ZERO, bin_expr.right_expr)
434435
end
435436
case result
436437
when Float
@@ -439,7 +440,7 @@ def calculate(left, right, operator, left_o, right_o, scope)
439440
end
440441
when Integer
441442
if result < MIN_INTEGER || result > MAX_INTEGER
442-
fail(Issues::NUMERIC_OVERFLOW, left_o.eContainer, {:value => result})
443+
fail(Issues::NUMERIC_OVERFLOW, bin_expr, {:value => result})
443444
end
444445
end
445446
result

Diff for: lib/puppet/pops/evaluator/external_syntax_support.rb

+1-2
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ def assert_external_syntax(scope, result, syntax, reference_expr)
2020
# Call checker and give it the location information from the expression
2121
# (as opposed to where the heredoc tag is (somewhere on the line above)).
2222
acceptor = Puppet::Pops::Validation::Acceptor.new()
23-
source_pos = Puppet::Pops::Utils.find_closest_positioned(reference_expr)
24-
checker.check(result, syntax, acceptor, source_pos)
23+
checker.check(result, syntax, acceptor, reference_expr)
2524

2625
if acceptor.error_count > 0
2726
checker_message = "Invalid produced text having syntax: '#{syntax}'."

Diff for: lib/puppet/pops/evaluator/runtime3_support.rb

+1-15
Original file line numberDiff line numberDiff line change
@@ -439,21 +439,7 @@ def is_boolean? x
439439
end
440440

441441
def extract_file_line(o)
442-
positioned = find_closest_with_offset(o)
443-
unless positioned.nil?
444-
locator = Adapters::SourcePosAdapter.find_locator(positioned)
445-
return [locator.file, locator.line_for_offset(positioned.offset)] unless locator.nil?
446-
end
447-
[nil, -1]
448-
end
449-
450-
def find_closest_with_offset(o)
451-
if o.offset.nil?
452-
c = o.eContainer
453-
c.nil? ? nil : find_closest_with_offset(c)
454-
else
455-
o
456-
end
442+
o.is_a?(Model::Positioned) ? [o.file, o.line] : [nil, -1]
457443
end
458444

459445
# Creates a diagnostic producer

Diff for: lib/puppet/pops/loader/type_definition_instantiator.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ def self.create_from_model(type_definition, loader)
5151
loader.set_entry(
5252
typed_name,
5353
type,
54-
Adapters::SourcePosAdapter.adapt(type_definition).to_uri)
54+
type_definition.locator.to_uri(type_definition))
5555
type
5656
end
5757

0 commit comments

Comments
 (0)